From 143e503d4325fe72074e85d8cf6f0a54ef99bf80 Mon Sep 17 00:00:00 2001 From: "Jeffrey C. Ollie" Date: Thu, 1 Aug 2024 01:25:17 -0500 Subject: [PATCH 01/12] gtk: allow running in the background This patch fixes #2010 by implementing `quit-after-last-window-closed` for the GTK apprt. It also adds the ability for the GTK apprt to exit after a delay once all surfaces have been closed and adds the ability to start Ghostty without opening an initial window. --- src/apprt/gtk/App.zig | 71 ++++++++++-- src/config/Config.zig | 264 ++++++++++++++++++++++++++++++++++++++++-- 2 files changed, 317 insertions(+), 18 deletions(-) diff --git a/src/apprt/gtk/App.zig b/src/apprt/gtk/App.zig index 60fefc6de..ef500fe44 100644 --- a/src/apprt/gtk/App.zig +++ b/src/apprt/gtk/App.zig @@ -253,10 +253,14 @@ pub fn init(core_app: *CoreApp, opts: Options) !App { break :x11_xkb try x11.Xkb.init(display); }; - // This just calls the "activate" signal but its part of the normal - // startup routine so we just call it: + // This just calls the `activate` signal but its part of the normal startup + // routine so we just call it, but only if the config allows it (this allows + // for launching Ghostty in the "background" without immediately opening + // a window) + // // https://gitlab.gnome.org/GNOME/glib/-/blob/bd2ccc2f69ecfd78ca3f34ab59e42e2b462bad65/gio/gapplication.c#L2302 - c.g_application_activate(gapp); + if (config.@"initial-window") + c.g_application_activate(gapp); // Register for dbus events if (c.g_application_get_dbus_connection(gapp)) |dbus_connection| { @@ -277,6 +281,10 @@ pub fn init(core_app: *CoreApp, opts: Options) !App { const css_provider = c.gtk_css_provider_new(); try loadRuntimeCss(&config, css_provider); + // Run a small no-op function every 500 milliseconds so that we don't get + // stuck in g_main_context_iteration forever if there are no open surfaces. + _ = c.g_timeout_add(500, gtkTimeout, null); + return .{ .core_app = core_app, .app = app, @@ -293,6 +301,12 @@ pub fn init(core_app: *CoreApp, opts: Options) !App { }; } +// This timeout function is run periodically so that we don't get stuck in +// g_main_context_iteration forever if there are no open surfaces. +pub fn gtkTimeout(_: ?*anyopaque) callconv(.C) c.gboolean { + return 1; +} + // Terminate the application. The application will not be restarted after // this so all global state can be cleaned up. pub fn terminate(self: *App) void { @@ -463,6 +477,9 @@ pub fn run(self: *App) !void { self.transient_cgroup_base = path; } else log.debug("cgroup isoation disabled config={}", .{self.config.@"linux-cgroup"}); + // The last instant that one or more surfaces were open + var last_one = try std.time.Instant.now(); + // Setup our menu items self.initActions(); self.initMenu(); @@ -478,9 +495,48 @@ pub fn run(self: *App) !void { while (self.running) { _ = c.g_main_context_iteration(self.ctx, 1); - // Tick the terminal app + // Tick the terminal app and see if we should quit. const should_quit = try self.core_app.tick(self); - if (should_quit or self.core_app.surfaces.items.len == 0) self.quit(); + + // If there are one or more surfaces open, update the timer. + if (self.core_app.surfaces.items.len > 0) last_one = try std.time.Instant.now(); + + const q = 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 there are no surfaces check to see if we should stay in the + // background or not. + if (self.core_app.surfaces.items.len == 0) { + if (self.config.@"quit-after-last-window-closed") { + // If the background timeout is not zero, check to see if + // the timeout has elapsed. + if (self.config.@"quit-after-last-window-closed-delay".duration != 0) { + const now = try std.time.Instant.now(); + + if (now.since(last_one) > self.config.@"quit-after-last-window-closed-delay".duration) { + log.info("timeout elapsed", .{}); + // The timeout has elapsed, quit. + break :q true; + } + + // Not enough time has elapsed, don't quit. + break :q false; + } + + // `quit-after-last-window-closed-delay` is zero, don't quit. + break :q false; + } + + break :q false; + } + + // there's at least one surface open, don't quit. + break :q false; + }; + + if (q) self.quit(); } } @@ -598,9 +654,8 @@ fn gtkQuitConfirmation( self.quitNow(); } -/// This is called by the "activate" signal. This is sent on program -/// startup and also when a secondary instance launches and requests -/// a new window. +/// This is called by the `activate` signal. This is sent on program startup and +/// also when a secondary instance launches and requests a new window. fn gtkActivate(app: *c.GtkApplication, ud: ?*anyopaque) callconv(.C) void { _ = app; diff --git a/src/config/Config.zig b/src/config/Config.zig index 81799d167..50fdafd44 100644 --- a/src/config/Config.zig +++ b/src/config/Config.zig @@ -880,11 +880,46 @@ keybind: Keybinds = .{}, /// true. If set to false, surfaces will close without any confirmation. @"confirm-close-surface": bool = true, -/// Whether or not to quit after the last window is closed. This defaults to -/// false. Currently only supported on macOS. On Linux, the process always exits -/// after the last window is closed. +/// Whether or not to quit after the last surface is closed. This +/// defaults to `false`. On Linux, if this is `true`, Ghostty can delay +/// quitting fully for a configurable amount. See the documentation of +/// `quit-after-last-window-closed-delay` for more information. @"quit-after-last-window-closed": bool = false, +/// If `quit-after-last-window-closed` is `true`, this controls how long +/// Ghostty will stay running after the last open surface has been closed. If +/// `quit-after-last-window-closed-delay` is set to 0 ns Ghostty will remain +/// running indefinitely. The duration should be at least long enough to allow +/// Ghostty to initialize and open it's first window, but this is not enforced +/// nor will a warning be issued. The duration is specified as a series of +/// numbers followed by time units. Whitespace is allowed between numbers and +/// units. The allowed time units are as follows: +/// +/// * `y` - 365 SI days, or 8760 hours, or 31536000 seconds. No adjustments +/// are made for leap years or leap seconds. +/// * `d` - one SI day, or 86400 seconds. +/// * `h` - one hour, or 3600 seconds. +/// * `m` - one minute, or 60 seconds. +/// * `s` - one second. +/// * `ms` - one millisecond, or 0.001 second. +/// * `us` or `µs` - one microsecond, or 0.000001 second. +/// * `ns` - one nanosecond, or 0.000000001 second. +/// +/// Examples: +/// * `1h30m` +/// * `45s` +/// +/// The maximum value is `584y 49w 23h 34m 33s 709ms 551µs 615ns`. +/// +/// By default `quit-after-last-window-closed-delay` is set to `0 ns`. +/// +/// Only implemented on Linux. +@"quit-after-last-window-closed-delay": Duration = .{ .duration = 0 }, + +/// This controls whether an initial window is created when Ghostty is run. Only +/// implemented on Linux. +@"initial-window": bool = true, + /// Whether to enable shell integration auto-injection or not. Shell integration /// greatly enhances the terminal experience by enabling a number of features: /// @@ -1109,15 +1144,16 @@ keybind: Keybinds = .{}, /// must always be able to move themselves into an isolated cgroup. @"linux-cgroup-hard-fail": bool = false, -/// If true, the Ghostty GTK application will run in single-instance mode: -/// each new `ghostty` process launched will result in a new window if there -/// is already a running process. +/// If `true`, the Ghostty GTK application will run in single-instance mode: +/// each new `ghostty` process launched will result in a new window if there is +/// already a running process. /// -/// If false, each new ghostty process will launch a separate application. +/// If `false`, each new ghostty process will launch a separate application. /// -/// The default value is `desktop` which will default to `true` if Ghostty -/// detects it was launched from the `.desktop` file such as an app launcher. -/// If Ghostty is launched from the command line, it will default to `false`. +/// The default value is `detect` which will default to `true` if Ghostty +/// detects that it was launched from the `.desktop` file such as an app +/// launcher (like Gnome Shell) or by D-Bus activation. If Ghostty is launched +/// from the command line, it will default to `false`. /// /// Note that debug builds of Ghostty have a separate single-instance ID /// so you can test single instance without conflicting with release builds. @@ -3753,3 +3789,211 @@ pub const LinuxCgroup = enum { always, @"single-instance", }; + +pub const Duration = struct { + duration: u64 = 0, + + pub fn clone(self: *const @This(), _: Allocator) !@This() { + return .{ .duration = self.duration }; + } + + pub fn equal(self: @This(), other: @This()) bool { + return self.duration == other.duration; + } + + pub fn parseCLI(self: *@This(), _: Allocator, input: ?[]const u8) !void { + var remaining = input orelse { + self.duration = 0; + return; + }; + + const units = [_]struct { + name: []const u8, + factor: u64, + }{ + .{ .name = "y", .factor = 365 * std.time.ns_per_day }, + .{ .name = "w", .factor = std.time.ns_per_week }, + .{ .name = "d", .factor = std.time.ns_per_day }, + .{ .name = "h", .factor = std.time.ns_per_hour }, + .{ .name = "m", .factor = std.time.ns_per_min }, + .{ .name = "s", .factor = std.time.ns_per_s }, + .{ .name = "ms", .factor = std.time.ns_per_ms }, + .{ .name = "us", .factor = std.time.ns_per_us }, + .{ .name = "µs", .factor = std.time.ns_per_us }, + .{ .name = "ns", .factor = 1 }, + }; + + var value: ?u64 = null; + + while (remaining.len > 0) { + // Skip over whitespace before the number + while (remaining.len > 0 and std.ascii.isWhitespace(remaining[0])) { + remaining = remaining[1..]; + } + // There was whitespace at the end, that's OK + if (remaining.len == 0) break; + + // Find the longest number + const number = number: { + var prev_number: ?u64 = null; + var prev_remaining: ?[]const u8 = null; + for (1..remaining.len + 1) |index| { + prev_number = std.fmt.parseUnsigned(u64, remaining[0..index], 10) catch { + if (prev_remaining) |prev| remaining = prev; + break :number prev_number; + }; + prev_remaining = remaining[index..]; + } + if (prev_remaining) |prev| remaining = prev; + break :number prev_number; + } orelse return error.InvalidValue; + + // Skip over any whitespace between the number and the unit + while (remaining.len > 0 and std.ascii.isWhitespace(remaining[0])) { + remaining = remaining[1..]; + } + + // A number without a unit is invalid + if (remaining.len == 0) return error.InvalidValue; + + // Find the longest matching unit. Needs to be the longest matching + // to distinguish 'm' from 'ms'. + const factor = factor: { + var prev_factor: ?u64 = null; + var prev_index: ?usize = null; + for (1..remaining.len + 1) |index| { + const next_factor = next: { + for (units) |unit| { + if (std.mem.eql(u8, unit.name, remaining[0..index])) { + break :next unit.factor; + } + } + break :next null; + }; + if (next_factor) |next| { + prev_factor = next; + prev_index = index; + } + } + if (prev_index) |index| { + remaining = remaining[index..]; + } + break :factor prev_factor; + } orelse return error.InvalidValue; + + if (value) |v| + value = v + number * factor + else + value = number * factor; + } + + self.duration = value orelse 0; + } + + pub fn formatEntry(self: @This(), formatter: anytype) !void { + var buf: [64]u8 = undefined; + var fbs = std.io.fixedBufferStream(&buf); + const writer = fbs.writer(); + + var value = self.duration; + + const units = [_]struct { + name: []const u8, + factor: u64, + }{ + .{ .name = "y", .factor = 365 * std.time.ns_per_day }, + .{ .name = "w", .factor = std.time.ns_per_week }, + .{ .name = "d", .factor = std.time.ns_per_day }, + .{ .name = "h", .factor = std.time.ns_per_hour }, + .{ .name = "m", .factor = std.time.ns_per_min }, + .{ .name = "s", .factor = std.time.ns_per_s }, + .{ .name = "ms", .factor = std.time.ns_per_ms }, + .{ .name = "µs", .factor = std.time.ns_per_us }, + .{ .name = "ns", .factor = 1 }, + }; + + var i: usize = 0; + for (units) |unit| { + if (value > unit.factor) { + if (i > 0) writer.writeAll(" ") catch unreachable; + const remainder = value % unit.factor; + const quotient = (value - remainder) / unit.factor; + writer.print("{d}{s}", .{ quotient, unit.name }) catch unreachable; + value = remainder; + i += 1; + } + } + + try formatter.formatEntry([]const u8, fbs.getWritten()); + } +}; + +test "parse duration" { + var d: Duration = undefined; + + try d.parseCLI(std.testing.allocator, ""); + try std.testing.expectEqual(@as(u64, 0), d.duration); + + try d.parseCLI(std.testing.allocator, "0ns"); + try std.testing.expectEqual(@as(u64, 0), d.duration); + + try d.parseCLI(std.testing.allocator, "1ns"); + try std.testing.expectEqual(@as(u64, 1), d.duration); + + try d.parseCLI(std.testing.allocator, "100ns"); + try std.testing.expectEqual(@as(u64, 100), d.duration); + + try d.parseCLI(std.testing.allocator, "1µs"); + try std.testing.expectEqual(@as(u64, 1000), d.duration); + + try d.parseCLI(std.testing.allocator, "1µs1ns"); + try std.testing.expectEqual(@as(u64, 1001), d.duration); + + try d.parseCLI(std.testing.allocator, "1µs 1ns"); + try std.testing.expectEqual(@as(u64, 1001), d.duration); + + try d.parseCLI(std.testing.allocator, " 1µs1ns"); + try std.testing.expectEqual(@as(u64, 1001), d.duration); + + try d.parseCLI(std.testing.allocator, "1µs1ns "); + try std.testing.expectEqual(@as(u64, 1001), d.duration); + + try d.parseCLI(std.testing.allocator, "1y"); + try std.testing.expectEqual(@as(u64, 365 * std.time.ns_per_day), d.duration); + + try d.parseCLI(std.testing.allocator, "1d"); + try std.testing.expectEqual(@as(u64, std.time.ns_per_day), d.duration); + + try d.parseCLI(std.testing.allocator, "1h"); + try std.testing.expectEqual(@as(u64, std.time.ns_per_hour), d.duration); + + try d.parseCLI(std.testing.allocator, "1m"); + try std.testing.expectEqual(@as(u64, std.time.ns_per_min), d.duration); + + try d.parseCLI(std.testing.allocator, "1s"); + try std.testing.expectEqual(@as(u64, std.time.ns_per_s), d.duration); + + try d.parseCLI(std.testing.allocator, "1ms"); + try std.testing.expectEqual(@as(u64, std.time.ns_per_ms), d.duration); + + try d.parseCLI(std.testing.allocator, "30s"); + try std.testing.expectEqual(@as(u64, 30 * std.time.ns_per_s), d.duration); + + try d.parseCLI(std.testing.allocator, "584y 49w 23h 34m 33s 709ms 551µs 615ns"); + try std.testing.expectEqual(std.math.maxInt(u64), d.duration); + + try std.testing.expectError(error.InvalidValue, d.parseCLI(std.testing.allocator, "1")); + try std.testing.expectError(error.InvalidValue, d.parseCLI(std.testing.allocator, "s")); + try std.testing.expectError(error.InvalidValue, d.parseCLI(std.testing.allocator, "1x")); + try std.testing.expectError(error.InvalidValue, d.parseCLI(std.testing.allocator, "1 ")); +} + +test "format duration" { + const testing = std.testing; + var buf = std.ArrayList(u8).init(testing.allocator); + defer buf.deinit(); + + var p: Duration = .{ .duration = std.math.maxInt(u64) }; + try p.formatEntry(formatterpkg.entryFormatter("a", buf.writer())); + try std.testing.expectEqualStrings("a = 584y 49w 23h 34m 33s 709ms 551µs 615ns\n", buf.items); +} From 3d6ca14dc6b954eafdada4f54eab203dc7ed86e5 Mon Sep 17 00:00:00 2001 From: "Jeffrey C. Ollie" Date: Thu, 1 Aug 2024 10:51:08 -0500 Subject: [PATCH 02/12] make quit-after-last-window-closed-delay an optional --- src/apprt/gtk/App.zig | 7 +-- src/config/Config.zig | 138 +++++++++++++++++++++++++----------------- 2 files changed, 85 insertions(+), 60 deletions(-) diff --git a/src/apprt/gtk/App.zig b/src/apprt/gtk/App.zig index ef500fe44..b6ab12472 100644 --- a/src/apprt/gtk/App.zig +++ b/src/apprt/gtk/App.zig @@ -512,11 +512,10 @@ pub fn run(self: *App) !void { if (self.config.@"quit-after-last-window-closed") { // If the background timeout is not zero, check to see if // the timeout has elapsed. - if (self.config.@"quit-after-last-window-closed-delay".duration != 0) { + if (self.config.@"quit-after-last-window-closed-delay") |duration| { const now = try std.time.Instant.now(); - if (now.since(last_one) > self.config.@"quit-after-last-window-closed-delay".duration) { - log.info("timeout elapsed", .{}); + if (now.since(last_one) > duration.duration) { // The timeout has elapsed, quit. break :q true; } @@ -525,7 +524,7 @@ pub fn run(self: *App) !void { break :q false; } - // `quit-after-last-window-closed-delay` is zero, don't quit. + // `quit-after-last-window-closed-delay` is not set, don't quit. break :q false; } diff --git a/src/config/Config.zig b/src/config/Config.zig index 50fdafd44..d8b356ca9 100644 --- a/src/config/Config.zig +++ b/src/config/Config.zig @@ -887,13 +887,13 @@ keybind: Keybinds = .{}, @"quit-after-last-window-closed": bool = false, /// If `quit-after-last-window-closed` is `true`, this controls how long -/// Ghostty will stay running after the last open surface has been closed. If -/// `quit-after-last-window-closed-delay` is set to 0 ns Ghostty will remain -/// running indefinitely. The duration should be at least long enough to allow -/// Ghostty to initialize and open it's first window, but this is not enforced -/// nor will a warning be issued. The duration is specified as a series of -/// numbers followed by time units. Whitespace is allowed between numbers and -/// units. The allowed time units are as follows: +/// Ghostty will stay running after the last open surface has been closed. If +/// `quit-after-last-window-closed-delay` is unset Ghostty will remain running +/// indefinitely. The duration should be at least long enough to allow Ghostty +/// to initialize and open it's first window, but this is not enforced nor +/// will a warning be issued. The duration is specified as a series of numbers +/// followed by time units. Whitespace is allowed between numbers and units. The +/// allowed time units are as follows: /// /// * `y` - 365 SI days, or 8760 hours, or 31536000 seconds. No adjustments /// are made for leap years or leap seconds. @@ -911,10 +911,10 @@ keybind: Keybinds = .{}, /// /// The maximum value is `584y 49w 23h 34m 33s 709ms 551µs 615ns`. /// -/// By default `quit-after-last-window-closed-delay` is set to `0 ns`. +/// By default `quit-after-last-window-closed-delay` is unset. /// /// Only implemented on Linux. -@"quit-after-last-window-closed-delay": Duration = .{ .duration = 0 }, +@"quit-after-last-window-closed-delay": ?Duration = null, /// This controls whether an initial window is created when Ghostty is run. Only /// implemented on Linux. @@ -3801,11 +3801,8 @@ pub const Duration = struct { return self.duration == other.duration; } - pub fn parseCLI(self: *@This(), _: Allocator, input: ?[]const u8) !void { - var remaining = input orelse { - self.duration = 0; - return; - }; + pub fn parseCLI(input: ?[]const u8) !Duration { + var remaining = input orelse return error.ValueRequired; const units = [_]struct { name: []const u8, @@ -3887,7 +3884,7 @@ pub const Duration = struct { value = number * factor; } - self.duration = value orelse 0; + return if (value) |v| .{ .duration = v } else error.ValueRequired; } pub fn formatEntry(self: @This(), formatter: anytype) !void { @@ -3929,63 +3926,92 @@ pub const Duration = struct { }; test "parse duration" { - var d: Duration = undefined; + { + const d = try Duration.parseCLI("0ns"); + try std.testing.expectEqual(@as(u64, 0), d.duration); + } - try d.parseCLI(std.testing.allocator, ""); - try std.testing.expectEqual(@as(u64, 0), d.duration); + { + const d = try Duration.parseCLI("1ns"); + try std.testing.expectEqual(@as(u64, 1), d.duration); + } - try d.parseCLI(std.testing.allocator, "0ns"); - try std.testing.expectEqual(@as(u64, 0), d.duration); + { + const d = try Duration.parseCLI("100ns"); + try std.testing.expectEqual(@as(u64, 100), d.duration); + } - try d.parseCLI(std.testing.allocator, "1ns"); - try std.testing.expectEqual(@as(u64, 1), d.duration); + { + const d = try Duration.parseCLI("1µs"); + try std.testing.expectEqual(@as(u64, 1000), d.duration); + } - try d.parseCLI(std.testing.allocator, "100ns"); - try std.testing.expectEqual(@as(u64, 100), d.duration); + { + const d = try Duration.parseCLI("1µs1ns"); + try std.testing.expectEqual(@as(u64, 1001), d.duration); + } - try d.parseCLI(std.testing.allocator, "1µs"); - try std.testing.expectEqual(@as(u64, 1000), d.duration); + { + const d = try Duration.parseCLI("1µs 1ns"); + try std.testing.expectEqual(@as(u64, 1001), d.duration); + } - try d.parseCLI(std.testing.allocator, "1µs1ns"); - try std.testing.expectEqual(@as(u64, 1001), d.duration); + { + const d = try Duration.parseCLI(" 1µs1ns"); + try std.testing.expectEqual(@as(u64, 1001), d.duration); + } - try d.parseCLI(std.testing.allocator, "1µs 1ns"); - try std.testing.expectEqual(@as(u64, 1001), d.duration); + { + const d = try Duration.parseCLI("1µs1ns "); + try std.testing.expectEqual(@as(u64, 1001), d.duration); + } - try d.parseCLI(std.testing.allocator, " 1µs1ns"); - try std.testing.expectEqual(@as(u64, 1001), d.duration); + { + const d = try Duration.parseCLI("1y"); + try std.testing.expectEqual(@as(u64, 365 * std.time.ns_per_day), d.duration); + } - try d.parseCLI(std.testing.allocator, "1µs1ns "); - try std.testing.expectEqual(@as(u64, 1001), d.duration); + { + const d = try Duration.parseCLI("1d"); + try std.testing.expectEqual(@as(u64, std.time.ns_per_day), d.duration); + } - try d.parseCLI(std.testing.allocator, "1y"); - try std.testing.expectEqual(@as(u64, 365 * std.time.ns_per_day), d.duration); + { + const d = try Duration.parseCLI("1h"); + try std.testing.expectEqual(@as(u64, std.time.ns_per_hour), d.duration); + } - try d.parseCLI(std.testing.allocator, "1d"); - try std.testing.expectEqual(@as(u64, std.time.ns_per_day), d.duration); + { + const d = try Duration.parseCLI("1m"); + try std.testing.expectEqual(@as(u64, std.time.ns_per_min), d.duration); + } - try d.parseCLI(std.testing.allocator, "1h"); - try std.testing.expectEqual(@as(u64, std.time.ns_per_hour), d.duration); + { + const d = try Duration.parseCLI("1s"); + try std.testing.expectEqual(@as(u64, std.time.ns_per_s), d.duration); + } - try d.parseCLI(std.testing.allocator, "1m"); - try std.testing.expectEqual(@as(u64, std.time.ns_per_min), d.duration); + { + const d = try Duration.parseCLI("1ms"); + try std.testing.expectEqual(@as(u64, std.time.ns_per_ms), d.duration); + } - try d.parseCLI(std.testing.allocator, "1s"); - try std.testing.expectEqual(@as(u64, std.time.ns_per_s), d.duration); + { + const d = try Duration.parseCLI("30s"); + try std.testing.expectEqual(@as(u64, 30 * std.time.ns_per_s), d.duration); + } - try d.parseCLI(std.testing.allocator, "1ms"); - try std.testing.expectEqual(@as(u64, std.time.ns_per_ms), d.duration); + { + const d = try Duration.parseCLI("584y 49w 23h 34m 33s 709ms 551µs 615ns"); + try std.testing.expectEqual(std.math.maxInt(u64), d.duration); + } - try d.parseCLI(std.testing.allocator, "30s"); - try std.testing.expectEqual(@as(u64, 30 * std.time.ns_per_s), d.duration); - - try d.parseCLI(std.testing.allocator, "584y 49w 23h 34m 33s 709ms 551µs 615ns"); - try std.testing.expectEqual(std.math.maxInt(u64), d.duration); - - try std.testing.expectError(error.InvalidValue, d.parseCLI(std.testing.allocator, "1")); - try std.testing.expectError(error.InvalidValue, d.parseCLI(std.testing.allocator, "s")); - try std.testing.expectError(error.InvalidValue, d.parseCLI(std.testing.allocator, "1x")); - try std.testing.expectError(error.InvalidValue, d.parseCLI(std.testing.allocator, "1 ")); + try std.testing.expectError(error.ValueRequired, Duration.parseCLI(null)); + try std.testing.expectError(error.ValueRequired, Duration.parseCLI("")); + try std.testing.expectError(error.InvalidValue, Duration.parseCLI("1")); + try std.testing.expectError(error.InvalidValue, Duration.parseCLI("s")); + try std.testing.expectError(error.InvalidValue, Duration.parseCLI("1x")); + try std.testing.expectError(error.InvalidValue, Duration.parseCLI("1 ")); } test "format duration" { From ec0f90d1b6215eeee41a5bfcf7cd672eecc75478 Mon Sep 17 00:00:00 2001 From: "Jeffrey C. Ollie" Date: Thu, 1 Aug 2024 14:49:02 -0500 Subject: [PATCH 03/12] Improve quit timers. Instead of "polling" to see if a quit timer has expired, start a single timer that expires after the confiugred delay when no more surfaces are open. That timer can be cancelled if necessary. --- src/App.zig | 4 ++ src/apprt/gtk/App.zig | 94 ++++++++++++++++++++++++------------------- src/main_ghostty.zig | 5 +++ 3 files changed, 62 insertions(+), 41 deletions(-) diff --git a/src/App.zig b/src/App.zig index 314d0b25b..69145ee58 100644 --- a/src/App.zig +++ b/src/App.zig @@ -134,6 +134,8 @@ pub fn updateConfig(self: *App, config: *const Config) !void { /// The surface must be from the pool. pub fn addSurface(self: *App, rt_surface: *apprt.Surface) !void { try self.surfaces.append(self.alloc, rt_surface); + + if (@hasDecl(apprt.App, "cancelQuitTimer")) rt_surface.app.cancelQuitTimer(); } /// Delete the surface from the known surface list. This will NOT call the @@ -158,6 +160,8 @@ pub fn deleteSurface(self: *App, rt_surface: *apprt.Surface) void { i += 1; } + + if (@hasDecl(apprt.App, "startQuitTimer") and self.surfaces.items.len == 0) rt_surface.app.startQuitTimer(); } /// The last focused surface. This is only valid while on the main thread diff --git a/src/apprt/gtk/App.zig b/src/apprt/gtk/App.zig index b6ab12472..80ea89ca9 100644 --- a/src/apprt/gtk/App.zig +++ b/src/apprt/gtk/App.zig @@ -76,6 +76,12 @@ transient_cgroup_base: ?[]const u8 = null, /// CSS Provider for any styles based on ghostty configuration values css_provider: *c.GtkCssProvider, +/// GLib source for tracking quit timer. +quit_timer_source: ?c.guint = null, + +/// If there is a quit timer, has it expired? +quit_timer_expired: bool = false, + pub fn init(core_app: *CoreApp, opts: Options) !App { _ = opts; @@ -281,10 +287,6 @@ pub fn init(core_app: *CoreApp, opts: Options) !App { const css_provider = c.gtk_css_provider_new(); try loadRuntimeCss(&config, css_provider); - // Run a small no-op function every 500 milliseconds so that we don't get - // stuck in g_main_context_iteration forever if there are no open surfaces. - _ = c.g_timeout_add(500, gtkTimeout, null); - return .{ .core_app = core_app, .app = app, @@ -301,12 +303,6 @@ pub fn init(core_app: *CoreApp, opts: Options) !App { }; } -// This timeout function is run periodically so that we don't get stuck in -// g_main_context_iteration forever if there are no open surfaces. -pub fn gtkTimeout(_: ?*anyopaque) callconv(.C) c.gboolean { - return 1; -} - // Terminate the application. The application will not be restarted after // this so all global state can be cleaned up. pub fn terminate(self: *App) void { @@ -477,9 +473,6 @@ pub fn run(self: *App) !void { self.transient_cgroup_base = path; } else log.debug("cgroup isoation disabled config={}", .{self.config.@"linux-cgroup"}); - // The last instant that one or more surfaces were open - var last_one = try std.time.Instant.now(); - // Setup our menu items self.initActions(); self.initMenu(); @@ -498,44 +491,63 @@ pub fn run(self: *App) !void { // Tick the terminal app and see if we should quit. const should_quit = try self.core_app.tick(self); - // If there are one or more surfaces open, update the timer. - if (self.core_app.surfaces.items.len > 0) last_one = try std.time.Instant.now(); - - const q = q: { + 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 there are no surfaces check to see if we should stay in the - // background or not. - if (self.core_app.surfaces.items.len == 0) { - if (self.config.@"quit-after-last-window-closed") { - // If the background timeout is not zero, check to see if - // the timeout has elapsed. - if (self.config.@"quit-after-last-window-closed-delay") |duration| { - const now = try std.time.Instant.now(); + // If we are configured to always stay running, don't quit. + if (!self.config.@"quit-after-last-window-closed") break :q false; - if (now.since(last_one) > duration.duration) { - // The timeout has elapsed, quit. - break :q true; - } - - // Not enough time has elapsed, don't quit. - break :q false; - } - - // `quit-after-last-window-closed-delay` is not set, don't quit. - break :q false; - } - - break :q false; + if (self.quit_timer_source) |_| { + // if the quit timer has expired, quit. + if (self.quit_timer_expired) break :q true; } - // there's at least one surface open, don't quit. + // There's no quit timer running, or it hasn't expired, don't quit. break :q false; }; - if (q) self.quit(); + if (must_quit) self.quit(); + } +} + +// This timeout function is started when no surfaces are open. It can be +// cancelled if a new surface is opened before the timer expires. +pub fn gtkQuitTimerExpired(ud: ?*anyopaque) callconv(.C) c.gboolean { + const self: *App = @ptrCast(@alignCast(ud)); + self.quit_timer_expired = true; + return c.FALSE; +} + +/// This will get called when there are no more open surfaces. +pub fn startQuitTimer(self: *App) void { + // Cancel any previous timeout. + if (self.quit_timer_source) |source| { + if (c.g_source_remove(source) == c.FALSE) + log.warn("unable to remove quit timer {d}", .{source}); + self.quit_timer_source = null; + } + + // This is a no-op unless we are configured to quit after last window is closed. + if (!self.config.@"quit-after-last-window-closed") return; + + // If a delay is configured, set a timeout function to quit after the delay. + if (self.config.@"quit-after-last-window-closed-delay") |duration| { + const t: c.guint = if (duration.duration > (std.math.maxInt(c.guint) * std.time.ns_per_ms)) + std.math.maxInt(c.guint) + else + @intCast(duration.duration / std.time.ns_per_ms); + self.quit_timer_source = c.g_timeout_add(t, gtkQuitTimerExpired, self); + } +} + +/// This will get called when a new surface gets opened. +pub fn cancelQuitTimer(self: *App) void { + if (self.quit_timer_source) |source| { + if (c.g_source_remove(source) == c.FALSE) + log.warn("unable to remove quit timer {d}", .{source}); + self.quit_timer_source = null; } } diff --git a/src/main_ghostty.zig b/src/main_ghostty.zig index 62b424f62..6354214d0 100644 --- a/src/main_ghostty.zig +++ b/src/main_ghostty.zig @@ -107,6 +107,11 @@ pub fn main() !MainReturn { var app_runtime = try apprt.App.init(app, .{}); defer app_runtime.terminate(); + // Since - by definition - there are no surfaces when first started, the + // quit timer may need to be started. The start timer will get cancelled if/ + // when the first surface is created. + if (@hasDecl(apprt.App, "startQuitTimer")) app_runtime.startQuitTimer(); + // Run the GUI event loop try app_runtime.run(); } From 3a4b236e6dfab4335f1a04f37f8e5b8ed43659ba Mon Sep 17 00:00:00 2001 From: "Jeffrey C. Ollie" Date: Fri, 2 Aug 2024 10:24:17 -0500 Subject: [PATCH 04/12] re-use code to cancel old timer --- src/apprt/gtk/App.zig | 6 +----- 1 file changed, 1 insertion(+), 5 deletions(-) diff --git a/src/apprt/gtk/App.zig b/src/apprt/gtk/App.zig index 80ea89ca9..f542e300e 100644 --- a/src/apprt/gtk/App.zig +++ b/src/apprt/gtk/App.zig @@ -523,11 +523,7 @@ pub fn gtkQuitTimerExpired(ud: ?*anyopaque) callconv(.C) c.gboolean { /// This will get called when there are no more open surfaces. pub fn startQuitTimer(self: *App) void { // Cancel any previous timeout. - if (self.quit_timer_source) |source| { - if (c.g_source_remove(source) == c.FALSE) - log.warn("unable to remove quit timer {d}", .{source}); - self.quit_timer_source = null; - } + self.cancelQuitTimer(); // This is a no-op unless we are configured to quit after last window is closed. if (!self.config.@"quit-after-last-window-closed") return; From 3aa1989620a2c285bda0b65ccff69ce34e98c896 Mon Sep 17 00:00:00 2001 From: "Jeffrey C. Ollie" Date: Fri, 2 Aug 2024 10:47:47 -0500 Subject: [PATCH 05/12] make Duration units a struct field to reduce duplication and enable table-driven unit tests --- src/config/Config.zig | 94 +++++++++++++------------------------------ 1 file changed, 28 insertions(+), 66 deletions(-) diff --git a/src/config/Config.zig b/src/config/Config.zig index d8b356ca9..9427e23fc 100644 --- a/src/config/Config.zig +++ b/src/config/Config.zig @@ -3793,6 +3793,24 @@ pub const LinuxCgroup = enum { pub const Duration = struct { duration: u64 = 0, + const units = [_]struct { + name: []const u8, + factor: u64, + }{ + // The order is important as the first factor that matches will be the + // default unit that is used for formatting. + .{ .name = "y", .factor = 365 * std.time.ns_per_day }, + .{ .name = "w", .factor = std.time.ns_per_week }, + .{ .name = "d", .factor = std.time.ns_per_day }, + .{ .name = "h", .factor = std.time.ns_per_hour }, + .{ .name = "m", .factor = std.time.ns_per_min }, + .{ .name = "s", .factor = std.time.ns_per_s }, + .{ .name = "ms", .factor = std.time.ns_per_ms }, + .{ .name = "µs", .factor = std.time.ns_per_us }, + .{ .name = "us", .factor = std.time.ns_per_us }, + .{ .name = "ns", .factor = 1 }, + }; + pub fn clone(self: *const @This(), _: Allocator) !@This() { return .{ .duration = self.duration }; } @@ -3804,22 +3822,6 @@ pub const Duration = struct { pub fn parseCLI(input: ?[]const u8) !Duration { var remaining = input orelse return error.ValueRequired; - const units = [_]struct { - name: []const u8, - factor: u64, - }{ - .{ .name = "y", .factor = 365 * std.time.ns_per_day }, - .{ .name = "w", .factor = std.time.ns_per_week }, - .{ .name = "d", .factor = std.time.ns_per_day }, - .{ .name = "h", .factor = std.time.ns_per_hour }, - .{ .name = "m", .factor = std.time.ns_per_min }, - .{ .name = "s", .factor = std.time.ns_per_s }, - .{ .name = "ms", .factor = std.time.ns_per_ms }, - .{ .name = "us", .factor = std.time.ns_per_us }, - .{ .name = "µs", .factor = std.time.ns_per_us }, - .{ .name = "ns", .factor = 1 }, - }; - var value: ?u64 = null; while (remaining.len > 0) { @@ -3827,6 +3829,7 @@ pub const Duration = struct { while (remaining.len > 0 and std.ascii.isWhitespace(remaining[0])) { remaining = remaining[1..]; } + // There was whitespace at the end, that's OK if (remaining.len == 0) break; @@ -3894,21 +3897,6 @@ pub const Duration = struct { var value = self.duration; - const units = [_]struct { - name: []const u8, - factor: u64, - }{ - .{ .name = "y", .factor = 365 * std.time.ns_per_day }, - .{ .name = "w", .factor = std.time.ns_per_week }, - .{ .name = "d", .factor = std.time.ns_per_day }, - .{ .name = "h", .factor = std.time.ns_per_hour }, - .{ .name = "m", .factor = std.time.ns_per_min }, - .{ .name = "s", .factor = std.time.ns_per_s }, - .{ .name = "ms", .factor = std.time.ns_per_ms }, - .{ .name = "µs", .factor = std.time.ns_per_us }, - .{ .name = "ns", .factor = 1 }, - }; - var i: usize = 0; for (units) |unit| { if (value > unit.factor) { @@ -3926,14 +3914,18 @@ pub const Duration = struct { }; test "parse duration" { - { - const d = try Duration.parseCLI("0ns"); + inline for (Duration.units) |unit| { + var buf: [16]u8 = undefined; + const t = try std.fmt.bufPrint(&buf, "0{s}", .{unit.name}); + const d = try Duration.parseCLI(t); try std.testing.expectEqual(@as(u64, 0), d.duration); } - { - const d = try Duration.parseCLI("1ns"); - try std.testing.expectEqual(@as(u64, 1), d.duration); + inline for (Duration.units) |unit| { + var buf: [16]u8 = undefined; + const t = try std.fmt.bufPrint(&buf, "1{s}", .{unit.name}); + const d = try Duration.parseCLI(t); + try std.testing.expectEqual(unit.factor, d.duration); } { @@ -3966,36 +3958,6 @@ test "parse duration" { try std.testing.expectEqual(@as(u64, 1001), d.duration); } - { - const d = try Duration.parseCLI("1y"); - try std.testing.expectEqual(@as(u64, 365 * std.time.ns_per_day), d.duration); - } - - { - const d = try Duration.parseCLI("1d"); - try std.testing.expectEqual(@as(u64, std.time.ns_per_day), d.duration); - } - - { - const d = try Duration.parseCLI("1h"); - try std.testing.expectEqual(@as(u64, std.time.ns_per_hour), d.duration); - } - - { - const d = try Duration.parseCLI("1m"); - try std.testing.expectEqual(@as(u64, std.time.ns_per_min), d.duration); - } - - { - const d = try Duration.parseCLI("1s"); - try std.testing.expectEqual(@as(u64, std.time.ns_per_s), d.duration); - } - - { - const d = try Duration.parseCLI("1ms"); - try std.testing.expectEqual(@as(u64, std.time.ns_per_ms), d.duration); - } - { const d = try Duration.parseCLI("30s"); try std.testing.expectEqual(@as(u64, 30 * std.time.ns_per_s), d.duration); From cf515c80d0c74fee52d57471767cf1c94e64289b Mon Sep 17 00:00:00 2001 From: "Jeffrey C. Ollie" Date: Fri, 2 Aug 2024 15:41:14 -0500 Subject: [PATCH 06/12] fix off-by-one error --- src/config/Config.zig | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/config/Config.zig b/src/config/Config.zig index 9427e23fc..b6b59d407 100644 --- a/src/config/Config.zig +++ b/src/config/Config.zig @@ -3899,7 +3899,7 @@ pub const Duration = struct { var i: usize = 0; for (units) |unit| { - if (value > unit.factor) { + if (value >= unit.factor) { if (i > 0) writer.writeAll(" ") catch unreachable; const remainder = value % unit.factor; const quotient = (value - remainder) / unit.factor; From d243ad6616a663d3222d290773edefbebdbadf56 Mon Sep 17 00:00:00 2001 From: "Jeffrey C. Ollie" Date: Fri, 2 Aug 2024 15:42:37 -0500 Subject: [PATCH 07/12] add a standard zig formatter to Duration and more testing --- src/config/Config.zig | 26 ++++++++++++++++++++------ 1 file changed, 20 insertions(+), 6 deletions(-) diff --git a/src/config/Config.zig b/src/config/Config.zig index b6b59d407..f99bcf9b3 100644 --- a/src/config/Config.zig +++ b/src/config/Config.zig @@ -3894,9 +3894,12 @@ pub const Duration = struct { var buf: [64]u8 = undefined; var fbs = std.io.fixedBufferStream(&buf); const writer = fbs.writer(); + try self.format("", .{}, writer); + try formatter.formatEntry([]const u8, fbs.getWritten()); + } + pub fn format(self: @This(), comptime _: []const u8, _: std.fmt.FormatOptions, writer: anytype) !void { var value = self.duration; - var i: usize = 0; for (units) |unit| { if (value >= unit.factor) { @@ -3908,8 +3911,6 @@ pub const Duration = struct { i += 1; } } - - try formatter.formatEntry([]const u8, fbs.getWritten()); } }; @@ -3976,9 +3977,22 @@ test "parse duration" { try std.testing.expectError(error.InvalidValue, Duration.parseCLI("1 ")); } -test "format duration" { - const testing = std.testing; - var buf = std.ArrayList(u8).init(testing.allocator); +test "test format" { + inline for (Duration.units) |unit| { + const d: Duration = .{ .duration = unit.factor }; + var actual_buf: [16]u8 = undefined; + const actual = try std.fmt.bufPrint(&actual_buf, "{}", .{d}); + var expected_buf: [16]u8 = undefined; + const expected = if (!std.mem.eql(u8, unit.name, "us")) + try std.fmt.bufPrint(&expected_buf, "1{s}", .{unit.name}) + else + "1µs"; + try std.testing.expectEqualSlices(u8, expected, actual); + } +} + +test "test entryFormatter" { + var buf = std.ArrayList(u8).init(std.testing.allocator); defer buf.deinit(); var p: Duration = .{ .duration = std.math.maxInt(u64) }; From 0e533af21a3af8a17d29505ffc57b50cd5f6a560 Mon Sep 17 00:00:00 2001 From: "Jeffrey C. Ollie" Date: Fri, 2 Aug 2024 15:43:02 -0500 Subject: [PATCH 08/12] add a warning if quit delay too short --- src/config/Config.zig | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/src/config/Config.zig b/src/config/Config.zig index f99bcf9b3..a03a823bf 100644 --- a/src/config/Config.zig +++ b/src/config/Config.zig @@ -2205,6 +2205,11 @@ pub fn finalize(self: *Config) !void { // If URLs are disabled, cut off the first link. The first link is // always the URL matcher. if (!self.@"link-url") self.link.links.items = self.link.links.items[1..]; + + if (self.@"quit-after-last-window-closed-delay") |duration| { + if (duration.duration < 5 * std.time.ns_per_s) + log.warn("quit-after-last-window-closed-delay is set to a very short value ({}), which might cause problems", .{duration}); + } } /// Callback for src/cli/args.zig to allow us to handle special cases From b87667c9503a3f9685595b4a6483efcae7f7a848 Mon Sep 17 00:00:00 2001 From: "Jeffrey C. Ollie" Date: Fri, 2 Aug 2024 16:17:10 -0500 Subject: [PATCH 09/12] document behavior when no initial window is created and a quit delay is set --- src/config/Config.zig | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/src/config/Config.zig b/src/config/Config.zig index a03a823bf..5a4535115 100644 --- a/src/config/Config.zig +++ b/src/config/Config.zig @@ -916,8 +916,11 @@ keybind: Keybinds = .{}, /// Only implemented on Linux. @"quit-after-last-window-closed-delay": ?Duration = null, -/// This controls whether an initial window is created when Ghostty is run. Only -/// implemented on Linux. +/// This controls whether an initial window is created when Ghostty +/// is run. Note that if `quit-after-last-window-closed` is `true` and +/// `quit-after-last-window-closed-delay` is set, setting `initial-window` to +/// `false` will mean that Ghostty will quit after the configured delay if no +/// window is ever created. Only implemented on Linux. @"initial-window": bool = true, /// Whether to enable shell integration auto-injection or not. Shell integration From e62bdbf8b75909df7811a6b6342f62d260429495 Mon Sep 17 00:00:00 2001 From: Mitchell Hashimoto Date: Sat, 3 Aug 2024 09:48:40 -0700 Subject: [PATCH 10/12] config: duration prevents overflow, added tests --- src/config/Config.zig | 65 ++++++++++++++++++++++++++++--------------- 1 file changed, 42 insertions(+), 23 deletions(-) diff --git a/src/config/Config.zig b/src/config/Config.zig index 5a4535115..e8580f404 100644 --- a/src/config/Config.zig +++ b/src/config/Config.zig @@ -881,19 +881,25 @@ keybind: Keybinds = .{}, @"confirm-close-surface": bool = true, /// Whether or not to quit after the last surface is closed. This -/// defaults to `false`. On Linux, if this is `true`, Ghostty can delay -/// quitting fully for a configurable amount. See the documentation of -/// `quit-after-last-window-closed-delay` for more information. +/// defaults to `false`. +/// +/// On Linux, if this is `true`, Ghostty can delay quitting fully until a +/// configurable amount of time has passed after the last window is closed. +/// See the documentation of `quit-after-last-window-closed-delay`. @"quit-after-last-window-closed": bool = false, -/// If `quit-after-last-window-closed` is `true`, this controls how long -/// Ghostty will stay running after the last open surface has been closed. If -/// `quit-after-last-window-closed-delay` is unset Ghostty will remain running -/// indefinitely. The duration should be at least long enough to allow Ghostty -/// to initialize and open it's first window, but this is not enforced nor -/// will a warning be issued. The duration is specified as a series of numbers -/// followed by time units. Whitespace is allowed between numbers and units. The -/// allowed time units are as follows: +/// Controls how long Ghostty will stay running after the last open surface has +/// been closed. This only has an effect if `quit-after-last-window-closed` is +/// also set to `true`. +/// +/// The minimum value for this configuration is `1s`. Any values lower than +/// this will be clamped to `1s`. +/// +/// The duration is specified as a series of numbers followed by time units. +/// Whitespace is allowed between numbers and units. Each number and unit will +/// be added together to form the total duration. +/// +/// The allowed time units are as follows: /// /// * `y` - 365 SI days, or 8760 hours, or 31536000 seconds. No adjustments /// are made for leap years or leap seconds. @@ -909,9 +915,16 @@ keybind: Keybinds = .{}, /// * `1h30m` /// * `45s` /// -/// The maximum value is `584y 49w 23h 34m 33s 709ms 551µs 615ns`. +/// Units can be repeated and will be added together. This means that +/// `1h1h` is equivalent to `2h`. This is confusing and should be avoided. +/// A future update may disallow this. /// -/// By default `quit-after-last-window-closed-delay` is unset. +/// The maximum value is `584y 49w 23h 34m 33s 709ms 551µs 615ns`. Any +/// value larger than this will be clamped to the maximum value. +/// +/// By default `quit-after-last-window-closed-delay` is unset and +/// Ghostty will quit immediately after the last window is closed if +/// `quit-after-last-window-closed` is `true`. /// /// Only implemented on Linux. @"quit-after-last-window-closed-delay": ?Duration = null, @@ -3799,6 +3812,7 @@ pub const LinuxCgroup = enum { }; pub const Duration = struct { + /// Duration in nanoseconds duration: u64 = 0, const units = [_]struct { @@ -3831,7 +3845,6 @@ pub const Duration = struct { var remaining = input orelse return error.ValueRequired; var value: ?u64 = null; - while (remaining.len > 0) { // Skip over whitespace before the number while (remaining.len > 0 and std.ascii.isWhitespace(remaining[0])) { @@ -3856,11 +3869,6 @@ pub const Duration = struct { break :number prev_number; } orelse return error.InvalidValue; - // Skip over any whitespace between the number and the unit - while (remaining.len > 0 and std.ascii.isWhitespace(remaining[0])) { - remaining = remaining[1..]; - } - // A number without a unit is invalid if (remaining.len == 0) return error.InvalidValue; @@ -3889,10 +3897,9 @@ pub const Duration = struct { break :factor prev_factor; } orelse return error.InvalidValue; - if (value) |v| - value = v + number * factor - else - value = number * factor; + // Add our time value to the total. Avoid overflow with saturating math. + const diff = std.math.mul(u64, number, factor) catch std.math.maxInt(u64); + value = (value orelse 0) +| diff; } return if (value) |v| .{ .duration = v } else error.ValueRequired; @@ -3977,6 +3984,18 @@ test "parse duration" { try std.testing.expectEqual(std.math.maxInt(u64), d.duration); } + // Overflow + { + const d = try Duration.parseCLI("600y"); + try std.testing.expectEqual(std.math.maxInt(u64), d.duration); + } + + // Repeated units + { + const d = try Duration.parseCLI("100ns100ns"); + try std.testing.expectEqual(@as(u64, 200), d.duration); + } + try std.testing.expectError(error.ValueRequired, Duration.parseCLI(null)); try std.testing.expectError(error.ValueRequired, Duration.parseCLI("")); try std.testing.expectError(error.InvalidValue, Duration.parseCLI("1")); From c5e2889369f979896e479ddec3d81dadbb4c5757 Mon Sep 17 00:00:00 2001 From: Mitchell Hashimoto Date: Sat, 3 Aug 2024 09:50:39 -0700 Subject: [PATCH 11/12] config: clarify comment --- src/config/Config.zig | 11 +++++++++-- 1 file changed, 9 insertions(+), 2 deletions(-) diff --git a/src/config/Config.zig b/src/config/Config.zig index e8580f404..fb5722fec 100644 --- a/src/config/Config.zig +++ b/src/config/Config.zig @@ -2222,9 +2222,16 @@ pub fn finalize(self: *Config) !void { // always the URL matcher. if (!self.@"link-url") self.link.links.items = self.link.links.items[1..]; + // We warn when the quit-after-last-window-closed-delay is set to a very + // short value because it can cause Ghostty to quit before the first + // window is even shown. if (self.@"quit-after-last-window-closed-delay") |duration| { - if (duration.duration < 5 * std.time.ns_per_s) - log.warn("quit-after-last-window-closed-delay is set to a very short value ({}), which might cause problems", .{duration}); + if (duration.duration < 5 * std.time.ns_per_s) { + log.warn( + "quit-after-last-window-closed-delay is set to a very short value ({}), which might cause problems", + .{duration}, + ); + } } } From 224f2d04911c573bc2332f8466924efc12447267 Mon Sep 17 00:00:00 2001 From: Mitchell Hashimoto Date: Sat, 3 Aug 2024 10:05:31 -0700 Subject: [PATCH 12/12] apprt/gtk: use tagged union for quit timer --- src/App.zig | 8 ++++++- src/apprt/gtk/App.zig | 49 ++++++++++++++++++++++++------------------- 2 files changed, 35 insertions(+), 22 deletions(-) diff --git a/src/App.zig b/src/App.zig index 69145ee58..f933b7126 100644 --- a/src/App.zig +++ b/src/App.zig @@ -135,6 +135,9 @@ pub fn updateConfig(self: *App, config: *const Config) !void { pub fn addSurface(self: *App, rt_surface: *apprt.Surface) !void { try self.surfaces.append(self.alloc, rt_surface); + // Since we have non-zero surfaces, we can cancel the quit timer. + // It is up to the apprt if there is a quit timer at all and if it + // should be canceled. if (@hasDecl(apprt.App, "cancelQuitTimer")) rt_surface.app.cancelQuitTimer(); } @@ -161,7 +164,10 @@ pub fn deleteSurface(self: *App, rt_surface: *apprt.Surface) void { i += 1; } - if (@hasDecl(apprt.App, "startQuitTimer") and self.surfaces.items.len == 0) rt_surface.app.startQuitTimer(); + // If we have no surfaces, we can start the quit timer. It is up to the + // apprt to determine if this is necessary. + if (@hasDecl(apprt.App, "startQuitTimer") and + self.surfaces.items.len == 0) rt_surface.app.startQuitTimer(); } /// The last focused surface. This is only valid while on the main thread diff --git a/src/apprt/gtk/App.zig b/src/apprt/gtk/App.zig index f542e300e..3767aa9a0 100644 --- a/src/apprt/gtk/App.zig +++ b/src/apprt/gtk/App.zig @@ -76,11 +76,12 @@ transient_cgroup_base: ?[]const u8 = null, /// CSS Provider for any styles based on ghostty configuration values css_provider: *c.GtkCssProvider, -/// GLib source for tracking quit timer. -quit_timer_source: ?c.guint = null, - -/// If there is a quit timer, has it expired? -quit_timer_expired: bool = false, +/// The timer used to quit the application after the last window is closed. +quit_timer: union(enum) { + off: void, + active: c.guint, + expired: void, +} = .{ .off = {} }, pub fn init(core_app: *CoreApp, opts: Options) !App { _ = opts; @@ -491,6 +492,7 @@ pub fn run(self: *App) !void { // Tick the terminal app and see if we should quit. const should_quit = 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. @@ -499,10 +501,8 @@ pub fn run(self: *App) !void { // If we are configured to always stay running, don't quit. if (!self.config.@"quit-after-last-window-closed") break :q false; - if (self.quit_timer_source) |_| { - // if the quit timer has expired, quit. - if (self.quit_timer_expired) break :q true; - } + // If the quit timer has expired, quit. + if (self.quit_timer == .expired) break :q true; // There's no quit timer running, or it hasn't expired, don't quit. break :q false; @@ -516,34 +516,41 @@ pub fn run(self: *App) !void { // cancelled if a new surface is opened before the timer expires. pub fn gtkQuitTimerExpired(ud: ?*anyopaque) callconv(.C) c.gboolean { const self: *App = @ptrCast(@alignCast(ud)); - self.quit_timer_expired = true; + self.quit_timer = .{ .expired = {} }; return c.FALSE; } /// This will get called when there are no more open surfaces. pub fn startQuitTimer(self: *App) void { - // Cancel any previous timeout. + // Cancel any previous timer. self.cancelQuitTimer(); // This is a no-op unless we are configured to quit after last window is closed. if (!self.config.@"quit-after-last-window-closed") return; // If a delay is configured, set a timeout function to quit after the delay. - if (self.config.@"quit-after-last-window-closed-delay") |duration| { - const t: c.guint = if (duration.duration > (std.math.maxInt(c.guint) * std.time.ns_per_ms)) - std.math.maxInt(c.guint) - else - @intCast(duration.duration / std.time.ns_per_ms); - self.quit_timer_source = c.g_timeout_add(t, gtkQuitTimerExpired, self); + if (self.config.@"quit-after-last-window-closed-delay") |v| { + const ms: u64 = std.math.divTrunc( + u64, + v.duration, + std.time.ns_per_ms, + ) catch std.math.maxInt(c.guint); + const t = std.math.cast(c.guint, ms) orelse std.math.maxInt(c.guint); + self.quit_timer = .{ .active = c.g_timeout_add(t, gtkQuitTimerExpired, self) }; } } /// This will get called when a new surface gets opened. pub fn cancelQuitTimer(self: *App) void { - if (self.quit_timer_source) |source| { - if (c.g_source_remove(source) == c.FALSE) - log.warn("unable to remove quit timer {d}", .{source}); - self.quit_timer_source = null; + switch (self.quit_timer) { + .off => {}, + .expired => self.quit_timer = .{ .off = {} }, + .active => |source| { + if (c.g_source_remove(source) == c.FALSE) { + log.warn("unable to remove quit timer source={d}", .{source}); + } + self.quit_timer = .{ .off = {} }; + }, } }