diff --git a/src/config/Config.zig b/src/config/Config.zig index 772458b57..ccdd51429 100644 --- a/src/config/Config.zig +++ b/src/config/Config.zig @@ -1707,53 +1707,6 @@ pub fn finalize(self: *Config) !void { } } - if (self.command) |command| command: { - // If the command has a space in it, we skip the check below because - // its probably a multi-argument command. These types of commands - // can contain full shell escapes and other characters and we don't - // want to reimplement that all here. The point of the check below - // is MOSTLY to find people who do `command = myshell` where "myshell" - // is installed by something like Homebrew and therefore isn't available - // to a login shell. We will do more robust checks in the future by - // simply checking if the command exited with an error. - if (std.mem.indexOf(u8, command, " ") != null) break :command; - - // If the path is not absolute then we want to expand it. We use our - // current path because our current path is what will be available - // to our termio launcher. - if (!std.fs.path.isAbsolute(command)) { - const expanded = Command.expandPath(alloc, command) catch |err| expanded: { - log.warn("failed to expand command path={s} err={}", .{ command, err }); - break :expanded null; - }; - - if (expanded) |v| { - self.command = v; - } else { - // If the command is not found on the path, we put an error - // but we still allow the command to remain unchanged and try - // to launch it later. - try self._errors.add(alloc, .{ - .message = try std.fmt.allocPrintZ( - alloc, - "command {s}: not found on PATH, use an absolute path", - .{command}, - ), - }); - } - } else { - std.fs.accessAbsolute(command, .{}) catch { - try self._errors.add(alloc, .{ - .message = try std.fmt.allocPrintZ( - alloc, - "command {s}: file not found", - .{command}, - ), - }); - }; - } - } - // If we have the special value "inherit" then set it to null which // does the same. In the future we should change to a tagged union. if (std.mem.eql(u8, wd, "inherit")) self.@"working-directory" = null; diff --git a/src/terminal/Terminal.zig b/src/terminal/Terminal.zig index 32f6ced86..af58f75da 100644 --- a/src/terminal/Terminal.zig +++ b/src/terminal/Terminal.zig @@ -425,7 +425,7 @@ fn clearPromptForResize(self: *Terminal) void { /// encoded as "\n". This omits any formatting such as fg/bg. /// /// The caller must free the string. -fn plainString(self: *Terminal, alloc: Allocator) ![]const u8 { +pub fn plainString(self: *Terminal, alloc: Allocator) ![]const u8 { return try self.screen.testString(alloc, .viewport); } @@ -715,9 +715,7 @@ pub fn invokeCharset( /// Print UTF-8 encoded string to the terminal. This string must be /// a single line, newlines and carriage returns and other control /// characters are not processed. -/// -/// This is not public because it is only used for tests rigt now. -fn printString(self: *Terminal, str: []const u8) !void { +pub fn printString(self: *Terminal, str: []const u8) !void { const view = try std.unicode.Utf8View.init(str); var it = view.iterator(); while (it.nextCodepoint()) |cp| try self.print(cp); diff --git a/src/termio/Exec.zig b/src/termio/Exec.zig index 4c204f792..ff7841647 100644 --- a/src/termio/Exec.zig +++ b/src/termio/Exec.zig @@ -38,6 +38,11 @@ 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. +const abnormal_runtime_threshold_ms = 250; + /// Allocator alloc: Allocator, @@ -230,13 +235,16 @@ pub fn threadEnter(self: *Exec, thread: *termio.Thread) !ThreadData { self.execFailedInChild() catch {}; std.os.exit(1); }; - errdefer self.subprocess.stop(); const pid = pid: { const command = self.subprocess.command orelse return error.ProcessNotStarted; break :pid command.pid orelse return error.ProcessNoPid; }; + // Track our process start time so we know how long it was + // running for. + const process_start = try std.time.Instant.now(); + // Create our pipe that we'll use to kill our read thread. // pipe[0] is the read end, pipe[1] is the write end. const pipe = try internal_os.pipe(); @@ -268,6 +276,7 @@ pub fn threadEnter(self: *Exec, thread: *termio.Thread) !ThreadData { .renderer_wakeup = self.renderer_wakeup, .renderer_mailbox = self.renderer_mailbox, .process = process, + .process_start = process_start, .data_stream = stream, .loop = &thread.loop, .terminal_stream = .{ @@ -535,9 +544,74 @@ 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 { + // Build up our command for the error message + const command = try std.mem.join( + self.alloc, + " ", + self.subprocess.args, + ); + defer self.alloc.free(command); + + // Modify the terminal to show our error message. This + // requires grabbing the renderer state lock. + self.renderer_state.mutex.lock(); + defer self.renderer_state.mutex.unlock(); + const t = 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(self.alloc); + defer self.alloc.free(viewport_str); + 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 = {} }); + try t.setAttribute(.{ .faint = {} }); + t.carriageReturn(); + try t.linefeed(); + try t.printString(command); + 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); +} + pub inline fn queueWrite(self: *Exec, data: []const u8, linefeed: bool) !void { const ev = self.data.?; + // If our process is exited then we send our surface a message + // about it but we don't queue any more writes. + if (ev.process_exited) { + _ = ev.surface_mailbox.push(.{ + .child_exited = {}, + }, .{ .forever = {} }); + return; + } + // We go through and chunk the data if necessary to fit into // our cached buffers that we can queue to the stream. var i: usize = 0; @@ -640,6 +714,7 @@ const EventData = struct { /// The process watcher process: xev.Process, + process_start: std.time.Instant, process_exited: bool = false, /// This is used for both waiting for the process to exit and then @@ -703,11 +778,54 @@ fn processExit( r: xev.Process.WaitError!u32, ) xev.CallbackAction { const code = r catch unreachable; - log.debug("child process exited status={}", .{code}); const ev = ev_.?; ev.process_exited = true; + // Determine how long the process was running for. + const runtime_ms: ?u64 = runtime: { + const process_end = std.time.Instant.now() catch break :runtime null; + const runtime_ns = process_end.since(ev.process_start); + const runtime_ms = runtime_ns / std.time.ns_per_ms; + break :runtime runtime_ms; + }; + log.debug( + "child process exited status={} runtime={}ms", + .{ 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.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; + } + + // 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 > 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. + _ = ev.writer_mailbox.push(.{ + .child_exited_abnormally = .{ + .code = code, + .runtime_ms = runtime, + }, + }, .{ .forever = {} }); + ev.writer_wakeup.notify() catch break :runtime; + + return .disarm; + } + // Notify our surface we want to close _ = ev.surface_mailbox.push(.{ .child_exited = {}, diff --git a/src/termio/Thread.zig b/src/termio/Thread.zig index 93faa38d5..516102f3f 100644 --- a/src/termio/Thread.zig +++ b/src/termio/Thread.zig @@ -186,6 +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(), .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 955fa44ed..e6c8fc76f 100644 --- a/src/termio/message.zig +++ b/src/termio/message.zig @@ -62,6 +62,15 @@ 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 { + code: u32, + runtime_ms: u64, + }, + /// Write where the data fits in the union. write_small: WriteReq.Small,