diff --git a/src/Surface.zig b/src/Surface.zig index 24b1e5be8..286d81383 100644 --- a/src/Surface.zig +++ b/src/Surface.zig @@ -237,6 +237,7 @@ const DerivedConfig = struct { /// For docs for these, see the associated config they are derived from. original_font_size: f32, keybind: configpkg.Keybinds, + abnormal_command_exit_runtime_ms: u32, clipboard_read: configpkg.ClipboardAccess, clipboard_write: configpkg.ClipboardAccess, clipboard_trim_trailing_spaces: bool, @@ -255,6 +256,7 @@ const DerivedConfig = struct { macos_option_as_alt: ?configpkg.OptionAsAlt, selection_clear_on_typing: bool, vt_kam_allowed: bool, + wait_after_command: bool, window_padding_top: u32, window_padding_bottom: u32, window_padding_left: u32, @@ -301,6 +303,7 @@ const DerivedConfig = struct { return .{ .original_font_size = config.@"font-size", .keybind = try config.keybind.clone(alloc), + .abnormal_command_exit_runtime_ms = config.@"abnormal-command-exit-runtime", .clipboard_read = config.@"clipboard-read", .clipboard_write = config.@"clipboard-write", .clipboard_trim_trailing_spaces = config.@"clipboard-trim-trailing-spaces", @@ -319,6 +322,7 @@ const DerivedConfig = struct { .macos_option_as_alt = config.@"macos-option-as-alt", .selection_clear_on_typing = config.@"selection-clear-on-typing", .vt_kam_allowed = config.@"vt-kam-allowed", + .wait_after_command = config.@"wait-after-command", .window_padding_top = config.@"window-padding-y".top_left, .window_padding_bottom = config.@"window-padding-y".bottom_right, .window_padding_left = config.@"window-padding-x".top_left, @@ -911,11 +915,7 @@ pub fn handleMessage(self: *Surface, msg: Message) !void { .close => self.close(), - // Close without confirmation. - .child_exited => { - self.child_exited = true; - self.close(); - }, + .child_exited => |v| self.childExited(v), .desktop_notification => |notification| { if (!self.config.desktop_notifications) { @@ -948,6 +948,136 @@ pub fn handleMessage(self: *Surface, msg: Message) !void { } } +fn childExited(self: *Surface, info: apprt.surface.Message.ChildExited) void { + // Mark our flag that we exited immediately + self.child_exited = true; + + // If our runtime was below some threshold then we assume that this + // was an abnormal exit and we show an error message. + if (info.runtime_ms <= self.config.abnormal_command_exit_runtime_ms) runtime: { + // On macOS, our exit code detection doesn't work, possibly + // because of our `login` wrapper. More investigation required. + if (comptime builtin.target.os.tag.isDarwin()) break :runtime; + + // If the exit code is 0 then we it was a good exit. + if (info.exit_code == 0) break :runtime; + log.warn("abnormal process exit detected, showing error message", .{}); + + // Update our terminal to note the abnormal exit. In the future we + // may want the apprt to handle this to show some native GUI element. + self.childExitedAbnormally(info) catch |err| { + log.err("error handling abnormal child exit err={}", .{err}); + return; + }; + + return; + } + + // We output a message so that the user knows whats going on and + // doesn't think their terminal just froze. We show this unconditionally + // on close even if `wait_after_command` is false and the surface closes + // immediately because if a user does an `undo` to restore a closed + // surface then they will see this message and know the process has + // completed. + terminal: { + self.renderer_state.mutex.lock(); + defer self.renderer_state.mutex.unlock(); + const t: *terminal.Terminal = self.renderer_state.terminal; + t.carriageReturn(); + t.linefeed() catch break :terminal; + t.printString("Process exited. Press any key to close the terminal.") catch + break :terminal; + t.modes.set(.cursor_visible, false); + } + + // Waiting after command we stop here. The terminal is updated, our + // state is updated, and now its up to the user to decide what to do. + if (self.config.wait_after_command) return; + + // If we aren't waiting after the command, then we exit immediately + // with no confirmation. + self.close(); +} + +/// Called when the child process exited abnormally. +fn childExitedAbnormally( + self: *Surface, + info: apprt.surface.Message.ChildExited, +) !void { + var arena = ArenaAllocator.init(self.alloc); + defer arena.deinit(); + const alloc = arena.allocator(); + + // Build up our command for the error message + const command = try std.mem.join(alloc, " ", switch (self.io.backend) { + .exec => |*exec| exec.subprocess.args, + }); + const runtime_str = try std.fmt.allocPrint(alloc, "{d} ms", .{info.runtime_ms}); + + self.renderer_state.mutex.lock(); + defer self.renderer_state.mutex.unlock(); + const t: *terminal.Terminal = self.renderer_state.terminal; + + // No matter what move the cursor back to the column 0. + t.carriageReturn(); + + // Reset styles + try t.setAttribute(.{ .unset = {} }); + + // If there is data in the viewport, we want to scroll down + // a little bit and write a horizontal rule before writing + // our message. This lets the use see the error message the + // command may have output. + const viewport_str = try t.plainString(alloc); + if (viewport_str.len > 0) { + try t.linefeed(); + for (0..t.cols) |_| try t.print(0x2501); + t.carriageReturn(); + try t.linefeed(); + try t.linefeed(); + } + + // Output our error message + try t.setAttribute(.{ .@"8_fg" = .bright_red }); + try t.setAttribute(.{ .bold = {} }); + try t.printString("Ghostty failed to launch the requested command:"); + try t.setAttribute(.{ .unset = {} }); + + t.carriageReturn(); + try t.linefeed(); + try t.linefeed(); + try t.printString(command); + try t.setAttribute(.{ .unset = {} }); + + t.carriageReturn(); + try t.linefeed(); + try t.linefeed(); + try t.printString("Runtime: "); + try t.setAttribute(.{ .@"8_fg" = .red }); + try t.printString(runtime_str); + try t.setAttribute(.{ .unset = {} }); + + // We don't print this on macOS because the exit code is always 0 + // due to the way we launch the process. + if (comptime !builtin.target.os.tag.isDarwin()) { + const exit_code_str = try std.fmt.allocPrint(alloc, "{d}", .{info.exit_code}); + t.carriageReturn(); + try t.linefeed(); + try t.printString("Exit Code: "); + try t.setAttribute(.{ .@"8_fg" = .red }); + try t.printString(exit_code_str); + try t.setAttribute(.{ .unset = {} }); + } + + t.carriageReturn(); + try t.linefeed(); + try t.linefeed(); + try t.printString("Press any key to close the window."); + + // Hide the cursor + t.modes.set(.cursor_visible, false); +} + /// Called when the terminal detects there is a password input prompt. fn passwordInput(self: *Surface, v: bool) !void { { @@ -1953,6 +2083,14 @@ pub fn keyCallback( if (self.io.terminal.modes.get(.disable_keyboard)) return .consumed; } + // If our process is exited and we press a key then we close the + // surface. We may want to eventually move this to the apprt rather + // than in core. + if (self.child_exited and event.action == .press) { + self.close(); + return .closed; + } + // If this input event has text, then we hide the mouse if configured. // We only do this on pressed events to avoid hiding the mouse when we // change focus due to a keybinding (i.e. switching tabs). diff --git a/src/apprt/surface.zig b/src/apprt/surface.zig index dce6a3a56..fcc67134b 100644 --- a/src/apprt/surface.zig +++ b/src/apprt/surface.zig @@ -43,8 +43,9 @@ pub const Message = union(enum) { 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 close, it may not. Additional details about the child + /// command are given in the `ChildExited` struct. + child_exited: ChildExited, /// Show a desktop notification. desktop_notification: struct { @@ -89,6 +90,11 @@ pub const Message = union(enum) { // This enum is a placeholder for future title styles. }; + + pub const ChildExited = struct { + exit_code: u32, + runtime_ms: u64, + }; }; /// A surface mailbox. diff --git a/src/termio/Exec.zig b/src/termio/Exec.zig index aed7cefb6..b8f838cf9 100644 --- a/src/termio/Exec.zig +++ b/src/termio/Exec.zig @@ -10,6 +10,7 @@ const Allocator = std.mem.Allocator; const ArenaAllocator = std.heap.ArenaAllocator; const posix = std.posix; const xev = @import("../global.zig").xev; +const apprt = @import("../apprt.zig"); const build_config = @import("../build_config.zig"); const configpkg = @import("../config.zig"); const crash = @import("../crash/main.zig"); @@ -153,8 +154,6 @@ pub fn threadEnter( // Setup our threadata backend state to be our own td.backend = .{ .exec = .{ .start = process_start, - .abnormal_runtime_threshold_ms = io.config.abnormal_runtime_threshold_ms, - .wait_after_command = io.config.wait_after_command, .write_stream = stream, .process = process, .read_thread = read_thread, @@ -273,83 +272,6 @@ pub fn resize( return try self.subprocess.resize(grid_size, screen_size); } -/// Called when the child process exited abnormally but before the surface -/// is notified. -pub fn childExitedAbnormally( - self: *Exec, - gpa: Allocator, - t: *terminal.Terminal, - exit_code: u32, - runtime_ms: u64, -) !void { - var arena = ArenaAllocator.init(gpa); - defer arena.deinit(); - const alloc = arena.allocator(); - - // Build up our command for the error message - const command = try std.mem.join(alloc, " ", self.subprocess.args); - const runtime_str = try std.fmt.allocPrint(alloc, "{d} ms", .{runtime_ms}); - - // No matter what move the cursor back to the column 0. - t.carriageReturn(); - - // Reset styles - try t.setAttribute(.{ .unset = {} }); - - // If there is data in the viewport, we want to scroll down - // a little bit and write a horizontal rule before writing - // our message. This lets the use see the error message the - // command may have output. - const viewport_str = try t.plainString(alloc); - if (viewport_str.len > 0) { - try t.linefeed(); - for (0..t.cols) |_| try t.print(0x2501); - t.carriageReturn(); - try t.linefeed(); - try t.linefeed(); - } - - // Output our error message - try t.setAttribute(.{ .@"8_fg" = .bright_red }); - try t.setAttribute(.{ .bold = {} }); - try t.printString("Ghostty failed to launch the requested command:"); - try t.setAttribute(.{ .unset = {} }); - - t.carriageReturn(); - try t.linefeed(); - try t.linefeed(); - try t.printString(command); - try t.setAttribute(.{ .unset = {} }); - - t.carriageReturn(); - try t.linefeed(); - try t.linefeed(); - try t.printString("Runtime: "); - try t.setAttribute(.{ .@"8_fg" = .red }); - try t.printString(runtime_str); - try t.setAttribute(.{ .unset = {} }); - - // We don't print this on macOS because the exit code is always 0 - // due to the way we launch the process. - if (comptime !builtin.target.os.tag.isDarwin()) { - const exit_code_str = try std.fmt.allocPrint(alloc, "{d}", .{exit_code}); - t.carriageReturn(); - try t.linefeed(); - try t.printString("Exit Code: "); - try t.setAttribute(.{ .@"8_fg" = .red }); - try t.printString(exit_code_str); - try t.setAttribute(.{ .unset = {} }); - } - - t.carriageReturn(); - try t.linefeed(); - try t.linefeed(); - try t.printString("Press any key to close the window."); - - // Hide the cursor - t.modes.set(.cursor_visible, false); -} - /// This outputs an error message when exec failed and we are the /// child process. This returns so the caller should probably exit /// after calling this. @@ -386,63 +308,13 @@ fn processExitCommon(td: *termio.Termio.ThreadData, exit_code: u32) void { .{ exit_code, runtime_ms orelse 0 }, ); - // If our runtime was below some threshold then we assume that this - // was an abnormal exit and we show an error message. - if (runtime_ms) |runtime| runtime: { - // On macOS, our exit code detection doesn't work, possibly - // because of our `login` wrapper. More investigation required. - if (comptime !builtin.target.os.tag.isDarwin()) { - // If our exit code is zero, then the command was successful - // and we don't ever consider it abnormal. - if (exit_code == 0) break :runtime; - } - - // Our runtime always has to be under the threshold to be - // considered abnormal. This is because a user can always - // manually do something like `exit 1` in their shell to - // force the exit code to be non-zero. We only want to detect - // abnormal exits that happen so quickly the user can't react. - if (runtime > execdata.abnormal_runtime_threshold_ms) break :runtime; - log.warn("abnormal process exit detected, showing error message", .{}); - - // Notify our main writer thread which has access to more - // information so it can show a better error message. - td.mailbox.send(.{ - .child_exited_abnormally = .{ - .exit_code = exit_code, - .runtime_ms = runtime, - }, - }, null); - td.mailbox.notify(); - - return; - } - - // We output a message so that the user knows whats going on and - // doesn't think their terminal just froze. We show this unconditionally - // on close even if `wait_after_command` is false and the surface closes - // immediately because if a user does an `undo` to restore a closed - // surface then they will see this message and know the process has - // completed. - terminal: { - td.renderer_state.mutex.lock(); - defer td.renderer_state.mutex.unlock(); - const t = td.renderer_state.terminal; - t.carriageReturn(); - t.linefeed() catch break :terminal; - t.printString("Process exited. Press any key to close the terminal.") catch - break :terminal; - t.modes.set(.cursor_visible, false); - } - - // If we're purposely waiting then we just return since the process - // exited flag is set to true. This allows the terminal window to remain - // open. - if (execdata.wait_after_command) return; - - // Notify our surface we want to close + // We always notify the surface immediately that the child has + // exited and some metadata about the exit. _ = td.surface_mailbox.push(.{ - .child_exited = {}, + .child_exited = .{ + .exit_code = exit_code, + .runtime_ms = runtime_ms orelse 0, + }, }, .{ .forever = {} }); } @@ -563,14 +435,8 @@ pub fn queueWrite( _ = self; const exec = &td.backend.exec; - // If our process is exited then we send our surface a message - // about it but we don't queue any more writes. - if (exec.exited) { - _ = td.surface_mailbox.push(.{ - .child_exited = {}, - }, .{ .forever = {} }); - return; - } + // If our process is exited then we don't send any more writes. + if (exec.exited) return; // We go through and chunk the data if necessary to fit into // our cached buffers that we can queue to the stream. @@ -658,17 +524,6 @@ pub const ThreadData = struct { start: std.time.Instant, exited: bool = false, - /// The number of milliseconds below which we consider a process - /// exit to be abnormal. This is used to show an error message - /// when the process exits too quickly. - abnormal_runtime_threshold_ms: u32, - - /// If true, do not immediately send a child exited message to the - /// surface to close the surface when the command exits. If this is - /// false we'll show a process exited message and wait for user input - /// to close the surface. - wait_after_command: bool, - /// The data stream is the main IO for the pty. write_stream: xev.Stream, diff --git a/src/termio/Termio.zig b/src/termio/Termio.zig index c474d55bb..865a2df86 100644 --- a/src/termio/Termio.zig +++ b/src/termio/Termio.zig @@ -168,8 +168,6 @@ pub const DerivedConfig = struct { foreground: configpkg.Config.Color, background: configpkg.Config.Color, osc_color_report_format: configpkg.Config.OSCColorReportFormat, - abnormal_runtime_threshold_ms: u32, - wait_after_command: bool, enquiry_response: []const u8, pub fn init( @@ -190,8 +188,6 @@ pub const DerivedConfig = struct { .foreground = config.foreground, .background = config.background, .osc_color_report_format = config.@"osc-color-report-format", - .abnormal_runtime_threshold_ms = config.@"abnormal-command-exit-runtime", - .wait_after_command = config.@"wait-after-command", .enquiry_response = try alloc.dupe(u8, config.@"enquiry-response"), // This has to be last so that we copy AFTER the arena allocations @@ -660,15 +656,6 @@ pub fn jumpToPrompt(self: *Termio, delta: isize) !void { try self.renderer_wakeup.notify(); } -/// Called when the child process exited abnormally but before -/// the surface is notified. -pub fn childExitedAbnormally(self: *Termio, exit_code: u32, runtime_ms: u64) !void { - self.renderer_state.mutex.lock(); - defer self.renderer_state.mutex.unlock(); - const t = self.renderer_state.terminal; - try self.backend.childExitedAbnormally(self.alloc, t, exit_code, runtime_ms); -} - /// Called when focus is gained or lost (when focus events are enabled) pub fn focusGained(self: *Termio, td: *ThreadData, focused: bool) !void { self.renderer_state.mutex.lock(); diff --git a/src/termio/Thread.zig b/src/termio/Thread.zig index 58a04f5a7..a701a29f8 100644 --- a/src/termio/Thread.zig +++ b/src/termio/Thread.zig @@ -311,7 +311,6 @@ fn drainMailbox( .jump_to_prompt => |v| try io.jumpToPrompt(v), .start_synchronized_output => self.startSynchronizedOutput(cb), .linefeed_mode => |v| self.flags.linefeed_mode = v, - .child_exited_abnormally => |v| try io.childExitedAbnormally(v.exit_code, v.runtime_ms), .focused => |v| try io.focusGained(data, v), .write_small => |v| try io.queueWrite( data, diff --git a/src/termio/backend.zig b/src/termio/backend.zig index 46ed3431c..280fcbde1 100644 --- a/src/termio/backend.zig +++ b/src/termio/backend.zig @@ -122,11 +122,7 @@ pub const ThreadData = union(Kind) { } pub fn changeConfig(self: *ThreadData, config: *termio.DerivedConfig) void { - switch (self.*) { - .exec => |*exec| { - exec.abnormal_runtime_threshold_ms = config.abnormal_runtime_threshold_ms; - exec.wait_after_command = config.wait_after_command; - }, - } + _ = self; + _ = config; } }; diff --git a/src/termio/message.zig b/src/termio/message.zig index 42767e109..e497a298f 100644 --- a/src/termio/message.zig +++ b/src/termio/message.zig @@ -1,6 +1,7 @@ const std = @import("std"); const assert = std.debug.assert; const Allocator = std.mem.Allocator; +const apprt = @import("../apprt.zig"); const renderer = @import("../renderer.zig"); const terminal = @import("../terminal/main.zig"); const termio = @import("../termio.zig"); @@ -58,15 +59,6 @@ pub const Message = union(enum) { /// Enable or disable linefeed mode (mode 20). linefeed_mode: bool, - /// The child exited abnormally. The termio state is marked - /// as process exited but the surface hasn't been notified to - /// close because termio can use this to update the terminal - /// with an error message. - child_exited_abnormally: struct { - exit_code: u32, - runtime_ms: u64, - }, - /// The surface gained or lost focus. focused: bool,