From 792284fb69fc5dea5f92d5efcd1b2786ca12f1a3 Mon Sep 17 00:00:00 2001 From: "Jeffrey C. Ollie" Date: Sat, 30 Dec 2023 22:24:25 -0600 Subject: [PATCH 1/5] Add exit code and runtime to abnormal exit error message. --- src/termio/Exec.zig | 31 +++++++++++++++++++++++++------ src/termio/Thread.zig | 2 +- src/termio/message.zig | 2 +- 3 files changed, 27 insertions(+), 8 deletions(-) diff --git a/src/termio/Exec.zig b/src/termio/Exec.zig index ff7841647..4394dbe59 100644 --- a/src/termio/Exec.zig +++ b/src/termio/Exec.zig @@ -546,7 +546,7 @@ pub fn jumpToPrompt(self: *Exec, delta: isize) !void { /// Called when the child process exited abnormally but before /// the surface is notified. -pub fn childExitedAbnormally(self: *Exec) !void { +pub fn childExitedAbnormally(self: *Exec, exit_code: u32, runtime_ms: u64) !void { // Build up our command for the error message const command = try std.mem.join( self.alloc, @@ -555,6 +555,11 @@ pub fn childExitedAbnormally(self: *Exec) !void { ); defer self.alloc.free(command); + const runtime_str = try std.fmt.allocPrint(self.alloc, "{d} ms", .{runtime_ms}); + defer self.alloc.free(runtime_str); + const exit_code_str = try std.fmt.allocPrint(self.alloc, "{d}", .{exit_code}); + defer self.alloc.free(exit_code_str); + // Modify the terminal to show our error message. This // requires grabbing the renderer state lock. self.renderer_state.mutex.lock(); @@ -586,14 +591,28 @@ pub fn childExitedAbnormally(self: *Exec) !void { try t.setAttribute(.{ .bold = {} }); try t.printString("Ghostty failed to launch the requested command:"); try t.setAttribute(.{ .unset = {} }); - try t.setAttribute(.{ .faint = {} }); t.carriageReturn(); try t.linefeed(); + try t.linefeed(); + try t.setAttribute(.{ .bold = {} }); 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" = .bright_red }); + try t.printString(runtime_str); + try t.setAttribute(.{ .unset = {} }); + t.carriageReturn(); + try t.linefeed(); + try t.printString("Exit code: "); + try t.setAttribute(.{ .@"8_fg" = .bright_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 @@ -777,7 +796,7 @@ fn processExit( _: *xev.Completion, r: xev.Process.WaitError!u32, ) xev.CallbackAction { - const code = r catch unreachable; + const exit_code = r catch unreachable; const ev = ev_.?; ev.process_exited = true; @@ -791,7 +810,7 @@ fn processExit( }; log.debug( "child process exited status={} runtime={}ms", - .{ code, runtime_ms orelse 0 }, + .{ exit_code, runtime_ms orelse 0 }, ); // If our runtime was below some threshold then we assume that this @@ -802,7 +821,7 @@ fn processExit( if (comptime !builtin.target.isDarwin()) { // If our exit code is zero, then the command was successful // and we don't ever consider it abnormal. - if (code == 0) break :runtime; + if (exit_code == 0) break :runtime; } // Our runtime always has to be under the threshold to be @@ -817,7 +836,7 @@ fn processExit( // information so it can show a better error message. _ = ev.writer_mailbox.push(.{ .child_exited_abnormally = .{ - .code = code, + .exit_code = exit_code, .runtime_ms = runtime, }, }, .{ .forever = {} }); diff --git a/src/termio/Thread.zig b/src/termio/Thread.zig index 516102f3f..61b2e3fba 100644 --- a/src/termio/Thread.zig +++ b/src/termio/Thread.zig @@ -186,7 +186,7 @@ fn drainMailbox(self: *Thread) !void { .jump_to_prompt => |v| try self.impl.jumpToPrompt(v), .start_synchronized_output => self.startSynchronizedOutput(), .linefeed_mode => |v| self.flags.linefeed_mode = v, - .child_exited_abnormally => try self.impl.childExitedAbnormally(), + .child_exited_abnormally => |v| try self.impl.childExitedAbnormally(v.exit_code, v.runtime_ms), .write_small => |v| try self.impl.queueWrite(v.data[0..v.len], self.flags.linefeed_mode), .write_stable => |v| try self.impl.queueWrite(v, self.flags.linefeed_mode), .write_alloc => |v| { diff --git a/src/termio/message.zig b/src/termio/message.zig index e6c8fc76f..ce6db8238 100644 --- a/src/termio/message.zig +++ b/src/termio/message.zig @@ -67,7 +67,7 @@ pub const Message = union(enum) { /// close because termio can use this to update the terminal /// with an error message. child_exited_abnormally: struct { - code: u32, + exit_code: u32, runtime_ms: u64, }, From 4ef8d099a7092c44035df283e605d1e617dbdee4 Mon Sep 17 00:00:00 2001 From: "Jeffrey C. Ollie" Date: Sat, 30 Dec 2023 22:52:47 -0600 Subject: [PATCH 2/5] Make the abnormal runtime threshold configurable. --- src/cli/args.zig | 6 ++++++ src/config/Config.zig | 5 +++++ src/termio/Exec.zig | 13 +++++++++++-- 3 files changed, 22 insertions(+), 2 deletions(-) diff --git a/src/cli/args.zig b/src/cli/args.zig index 99e6f7aa1..6444400a7 100644 --- a/src/cli/args.zig +++ b/src/cli/args.zig @@ -230,6 +230,12 @@ fn parseIntoField( 0, ) catch return error.InvalidValue, + u64 => std.fmt.parseInt( + u64, + value orelse return error.ValueRequired, + 0, + ) catch return error.InvalidValue, + f64 => std.fmt.parseFloat( f64, value orelse return error.ValueRequired, diff --git a/src/config/Config.zig b/src/config/Config.zig index ccdd51429..85d140492 100644 --- a/src/config/Config.zig +++ b/src/config/Config.zig @@ -387,6 +387,11 @@ palette: Palette = .{}, /// flag. For example: `ghostty -e fish --with --custom --args`. command: ?[]const u8 = null, +/// 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": u64 = 250, + /// Match a regular expression against the terminal text and associate /// clicking it with an action. This can be used to match URLs, file paths, /// etc. Actions can be opening using the system opener (i.e. "open" or diff --git a/src/termio/Exec.zig b/src/termio/Exec.zig index 4394dbe59..0db1c1e95 100644 --- a/src/termio/Exec.zig +++ b/src/termio/Exec.zig @@ -41,7 +41,7 @@ const disable_kitty_keyboard_protocol = apprt.runtime == apprt.glfw; /// 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. -const abnormal_runtime_threshold_ms = 250; +abnormal_runtime_threshold_ms: u64, /// Allocator alloc: Allocator, @@ -111,6 +111,7 @@ pub const DerivedConfig = struct { osc_color_report_format: configpkg.Config.OSCColorReportFormat, term: []const u8, grapheme_width_method: configpkg.Config.GraphemeWidthMethod, + abnormal_runtime_threshold_ms: u64, pub fn init( alloc_gpa: Allocator, @@ -129,6 +130,7 @@ pub const DerivedConfig = struct { .osc_color_report_format = config.@"osc-color-report-format", .term = config.term, .grapheme_width_method = config.@"grapheme-width-method", + .abnormal_runtime_threshold_ms = config.@"abnormal-runtime-threshold-ms", }; } @@ -207,6 +209,7 @@ pub fn init(alloc: Allocator, opts: termio.Options) !Exec { .background_color = config.background.toTerminalRGB(), .osc_color_report_format = config.osc_color_report_format, .data = null, + .abnormal_runtime_threshold_ms = opts.config.abnormal_runtime_threshold_ms, }; } @@ -304,6 +307,7 @@ pub fn threadEnter(self: *Exec, thread: *termio.Thread) !ThreadData { }, }, }, + .abnormal_runtime_threshold_ms = self.abnormal_runtime_threshold_ms, }; errdefer ev_data_ptr.deinit(self.alloc); @@ -764,6 +768,11 @@ const EventData = struct { /// this to determine if we need to default the window title. seen_title: 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: u64, + pub fn deinit(self: *EventData, alloc: Allocator) void { // Clear our write pools. We know we aren't ever going to do // any more IO since we stop our data stream below so we can just @@ -829,7 +838,7 @@ fn processExit( // 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 > abnormal_runtime_threshold_ms) break :runtime; + if (runtime > ev.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 From aef93a5420b71d6efcb78755d7bd309a94a445cb Mon Sep 17 00:00:00 2001 From: Mitchell Hashimoto Date: Sat, 30 Dec 2023 21:45:16 -0800 Subject: [PATCH 3/5] config: make abnormal runtime threshold a u32 --- src/config/Config.zig | 12 ++++++---- src/termio/Exec.zig | 55 +++++++++++++++++++++++-------------------- 2 files changed, 37 insertions(+), 30 deletions(-) diff --git a/src/config/Config.zig b/src/config/Config.zig index 85d140492..809eb427d 100644 --- a/src/config/Config.zig +++ b/src/config/Config.zig @@ -387,10 +387,14 @@ palette: Palette = .{}, /// flag. For example: `ghostty -e fish --with --custom --args`. command: ?[]const u8 = null, -/// 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": u64 = 250, +/// The number of milliseconds of runtime below which we consider a process +/// exit to be abnormal. This is used to show an error message when the +/// process exits too quickly. +/// +/// On Linux, this must be paired with a non-zero exit code. On macOS, +/// we allow any exit code because of the way shell processes are launched +/// via the login command. +@"abnormal-command-exit-runtime": u32 = 250, /// Match a regular expression against the terminal text and associate /// clicking it with an action. This can be used to match URLs, file paths, diff --git a/src/termio/Exec.zig b/src/termio/Exec.zig index 0db1c1e95..15ed97b3f 100644 --- a/src/termio/Exec.zig +++ b/src/termio/Exec.zig @@ -41,7 +41,7 @@ const disable_kitty_keyboard_protocol = apprt.runtime == apprt.glfw; /// 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: u64, +abnormal_runtime_threshold_ms: u32, /// Allocator alloc: Allocator, @@ -111,7 +111,7 @@ pub const DerivedConfig = struct { osc_color_report_format: configpkg.Config.OSCColorReportFormat, term: []const u8, grapheme_width_method: configpkg.Config.GraphemeWidthMethod, - abnormal_runtime_threshold_ms: u64, + abnormal_runtime_threshold_ms: u32, pub fn init( alloc_gpa: Allocator, @@ -130,7 +130,7 @@ pub const DerivedConfig = struct { .osc_color_report_format = config.@"osc-color-report-format", .term = config.term, .grapheme_width_method = config.@"grapheme-width-method", - .abnormal_runtime_threshold_ms = config.@"abnormal-runtime-threshold-ms", + .abnormal_runtime_threshold_ms = config.@"abnormal-command-exit-runtime", }; } @@ -551,18 +551,13 @@ pub fn jumpToPrompt(self: *Exec, delta: isize) !void { /// Called when the child process exited abnormally but before /// the surface is notified. pub fn childExitedAbnormally(self: *Exec, exit_code: u32, runtime_ms: u64) !void { - // Build up our command for the error message - const command = try std.mem.join( - self.alloc, - " ", - self.subprocess.args, - ); - defer self.alloc.free(command); + var arena = ArenaAllocator.init(self.alloc); + defer arena.deinit(); + const alloc = arena.allocator(); - const runtime_str = try std.fmt.allocPrint(self.alloc, "{d} ms", .{runtime_ms}); - defer self.alloc.free(runtime_str); - const exit_code_str = try std.fmt.allocPrint(self.alloc, "{d}", .{exit_code}); - defer self.alloc.free(exit_code_str); + // 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}); // Modify the terminal to show our error message. This // requires grabbing the renderer state lock. @@ -580,8 +575,7 @@ pub fn childExitedAbnormally(self: *Exec, exit_code: u32, runtime_ms: u64) !void // 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(self.alloc); - defer self.alloc.free(viewport_str); + const viewport_str = try t.plainString(alloc); if (viewport_str.len > 0) { try t.linefeed(); for (0..t.cols) |_| try t.print(0x2501); @@ -593,27 +587,36 @@ pub fn childExitedAbnormally(self: *Exec, exit_code: u32, runtime_ms: u64) !void // 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.printString("Ghostty failed to launch the requested command."); try t.setAttribute(.{ .unset = {} }); + t.carriageReturn(); try t.linefeed(); try t.linefeed(); - try t.setAttribute(.{ .bold = {} }); + try t.printString("Command: "); try t.printString(command); try t.setAttribute(.{ .unset = {} }); + t.carriageReturn(); try t.linefeed(); try t.linefeed(); - try t.printString("Runtime: "); + try t.printString("Runtime: "); try t.setAttribute(.{ .@"8_fg" = .bright_red }); try t.printString(runtime_str); try t.setAttribute(.{ .unset = {} }); - t.carriageReturn(); - try t.linefeed(); - try t.printString("Exit code: "); - try t.setAttribute(.{ .@"8_fg" = .bright_red }); - try t.printString(exit_code_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.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" = .bright_red }); + try t.printString(exit_code_str); + try t.setAttribute(.{ .unset = {} }); + } + t.carriageReturn(); try t.linefeed(); try t.linefeed(); @@ -771,7 +774,7 @@ const EventData = struct { /// 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: u64, + abnormal_runtime_threshold_ms: u32, pub fn deinit(self: *EventData, alloc: Allocator) void { // Clear our write pools. We know we aren't ever going to do From 134327c1a3a7fed74b713034fb42eb495edcee23 Mon Sep 17 00:00:00 2001 From: Mitchell Hashimoto Date: Sat, 30 Dec 2023 21:46:27 -0800 Subject: [PATCH 4/5] termio/exec: reorder member since we like alloc on top --- src/termio/Exec.zig | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/src/termio/Exec.zig b/src/termio/Exec.zig index 15ed97b3f..df0b6cb36 100644 --- a/src/termio/Exec.zig +++ b/src/termio/Exec.zig @@ -38,17 +38,17 @@ const c = @cImport({ /// correct granularity of events. const disable_kitty_keyboard_protocol = apprt.runtime == apprt.glfw; -/// 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, - /// Allocator alloc: Allocator, /// This is the pty fd created for the subcommand. subprocess: Subprocess, +/// 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, + /// The terminal emulator internal state. This is the abstract "terminal" /// that manages input, grid updating, etc. and is renderer-agnostic. It /// just stores internal state about a grid. From 0eee66f21b75368146fbcc7b0862b8c1cb96c0bf Mon Sep 17 00:00:00 2001 From: Mitchell Hashimoto Date: Sat, 30 Dec 2023 21:47:34 -0800 Subject: [PATCH 5/5] termio/exec: stylistic change on abnormal exit --- src/termio/Exec.zig | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/src/termio/Exec.zig b/src/termio/Exec.zig index df0b6cb36..16c25eb6b 100644 --- a/src/termio/Exec.zig +++ b/src/termio/Exec.zig @@ -587,13 +587,12 @@ pub fn childExitedAbnormally(self: *Exec, exit_code: u32, runtime_ms: u64) !void // 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.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.printString(command); try t.setAttribute(.{ .unset = {} }); @@ -601,7 +600,7 @@ pub fn childExitedAbnormally(self: *Exec, exit_code: u32, runtime_ms: u64) !void try t.linefeed(); try t.linefeed(); try t.printString("Runtime: "); - try t.setAttribute(.{ .@"8_fg" = .bright_red }); + try t.setAttribute(.{ .@"8_fg" = .red }); try t.printString(runtime_str); try t.setAttribute(.{ .unset = {} }); @@ -612,7 +611,7 @@ pub fn childExitedAbnormally(self: *Exec, exit_code: u32, runtime_ms: u64) !void t.carriageReturn(); try t.linefeed(); try t.printString("Exit Code: "); - try t.setAttribute(.{ .@"8_fg" = .bright_red }); + try t.setAttribute(.{ .@"8_fg" = .red }); try t.printString(exit_code_str); try t.setAttribute(.{ .unset = {} }); }