From 342b5e9d06a0f104acfc7d3216aa189eb2e27842 Mon Sep 17 00:00:00 2001 From: "Jeffrey C. Ollie" Date: Sun, 7 Jan 2024 23:47:39 -0600 Subject: [PATCH 1/6] implement enquiry --- docs/sequences/enq.md | 8 +++----- src/config/Config.zig | 4 ++++ src/termio/Exec.zig | 18 +++++++++++++++++- 3 files changed, 24 insertions(+), 6 deletions(-) diff --git a/docs/sequences/enq.md b/docs/sequences/enq.md index f6180d72f..15ccaf957 100644 --- a/docs/sequences/enq.md +++ b/docs/sequences/enq.md @@ -10,11 +10,9 @@ operator. ## Implementation Details -- ghostty always sends `""` - -## TODO - -- Make the answerback configurable +The answerback can be configured in the config file using the `enquiry-string` +configuration setting or on the command line using the `--enquiry-string` +parameter. The default is to send an empty string (`""`). ## References diff --git a/src/config/Config.zig b/src/config/Config.zig index 9881580b9..86c7e0c44 100644 --- a/src/config/Config.zig +++ b/src/config/Config.zig @@ -896,6 +896,10 @@ keybind: Keybinds = .{}, /// from working properly. https://github.com/vim/vim/pull/13211 fixes this. term: []const u8 = "xterm-ghostty", +/// String to send when we receive ENQ (0x05) from the command that we are +/// running. Defaults to "" if not set. +@"enquiry-string": ?[]const u8 = null, + /// This is set by the CLI parser for deinit. _arena: ?ArenaAllocator = null, diff --git a/src/termio/Exec.zig b/src/termio/Exec.zig index a694ebc19..66754f003 100644 --- a/src/termio/Exec.zig +++ b/src/termio/Exec.zig @@ -49,6 +49,10 @@ subprocess: Subprocess, /// when the process exits too quickly. abnormal_runtime_threshold_ms: u32, +/// Equiry string - the string to send back when we receive a ENQ (0x05) +/// from the process. +enquiry_string: ?[]const u8 = null, + /// 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. @@ -112,6 +116,7 @@ pub const DerivedConfig = struct { term: []const u8, grapheme_width_method: configpkg.Config.GraphemeWidthMethod, abnormal_runtime_threshold_ms: u32, + enquiry_string: ?[]const u8, pub fn init( alloc_gpa: Allocator, @@ -131,6 +136,7 @@ pub const DerivedConfig = struct { .term = config.term, .grapheme_width_method = config.@"grapheme-width-method", .abnormal_runtime_threshold_ms = config.@"abnormal-command-exit-runtime", + .enquiry_string = config.@"enquiry-string", }; } @@ -210,6 +216,7 @@ pub fn init(alloc: Allocator, opts: termio.Options) !Exec { .osc_color_report_format = config.osc_color_report_format, .data = null, .abnormal_runtime_threshold_ms = opts.config.abnormal_runtime_threshold_ms, + .enquiry_string = if (opts.config.enquiry_string) |s| try alloc.dupe(u8, s) else null, }; } @@ -218,6 +225,8 @@ pub fn deinit(self: *Exec) void { // Clean up our other members self.terminal.deinit(self.alloc); + + if (self.enquiry_string) |s| self.alloc.free(s); } pub fn threadEnter(self: *Exec, thread: *termio.Thread) !ThreadData { @@ -297,6 +306,7 @@ pub fn threadEnter(self: *Exec, thread: *termio.Thread) !ThreadData { .foreground_color = self.foreground_color, .background_color = self.background_color, .osc_color_report_format = self.osc_color_report_format, + .enquiry_string = if (self.enquiry_string) |s| try self.alloc.dupe(u8, s) else null, }, .parser = .{ @@ -431,6 +441,8 @@ pub fn changeConfig(self: *Exec, config: *DerivedConfig) !void { config.cursor_style, config.cursor_blink, ); + if (data.terminal_stream.handler.enquiry_string) |s| data.terminal_stream.handler.alloc.free(s); + data.terminal_stream.handler.enquiry_string = if (config.enquiry_string) |s| try data.terminal_stream.handler.alloc.dupe(u8, s) else null; } // Set the image size limits @@ -1695,9 +1707,12 @@ const StreamHandler = struct { osc_color_report_format: configpkg.Config.OSCColorReportFormat, + enquiry_string: ?[]const u8 = null, + pub fn deinit(self: *StreamHandler) void { self.apc.deinit(); self.dcs.deinit(); + if (self.enquiry_string) |s| self.alloc.free(s); } inline fn queueRender(self: *StreamHandler) !void { @@ -2390,7 +2405,8 @@ const StreamHandler = struct { } pub fn enquiry(self: *StreamHandler) !void { - self.messageWriter(.{ .write_stable = "" }); + log.debug("sending equiry string '{any}''", .{self.enquiry_string orelse ""}); + self.messageWriter(.{ .write_stable = self.enquiry_string orelse "" }); } pub fn scrollDown(self: *StreamHandler, count: usize) !void { From 3c2dfd4a842f839beffe003032f98a6d9a9283c5 Mon Sep 17 00:00:00 2001 From: "Jeffrey C. Ollie" Date: Tue, 9 Jan 2024 10:07:32 -0600 Subject: [PATCH 2/6] change name of config entry and variables, add lock for safety during config update --- docs/sequences/enq.md | 4 ++-- src/config/Config.zig | 2 +- src/termio/Exec.zig | 36 ++++++++++++++++++++++++------------ 3 files changed, 27 insertions(+), 15 deletions(-) diff --git a/docs/sequences/enq.md b/docs/sequences/enq.md index 15ccaf957..f7d3adfdb 100644 --- a/docs/sequences/enq.md +++ b/docs/sequences/enq.md @@ -10,8 +10,8 @@ operator. ## Implementation Details -The answerback can be configured in the config file using the `enquiry-string` -configuration setting or on the command line using the `--enquiry-string` +The answerback can be configured in the config file using the `enquiry-response` +configuration setting or on the command line using the `--enquiry-response` parameter. The default is to send an empty string (`""`). ## References diff --git a/src/config/Config.zig b/src/config/Config.zig index 86c7e0c44..61b3f5b6c 100644 --- a/src/config/Config.zig +++ b/src/config/Config.zig @@ -898,7 +898,7 @@ term: []const u8 = "xterm-ghostty", /// String to send when we receive ENQ (0x05) from the command that we are /// running. Defaults to "" if not set. -@"enquiry-string": ?[]const u8 = null, +@"enquiry-response": ?[]const u8 = null, /// This is set by the CLI parser for deinit. _arena: ?ArenaAllocator = null, diff --git a/src/termio/Exec.zig b/src/termio/Exec.zig index 66754f003..561eb76eb 100644 --- a/src/termio/Exec.zig +++ b/src/termio/Exec.zig @@ -51,7 +51,7 @@ abnormal_runtime_threshold_ms: u32, /// Equiry string - the string to send back when we receive a ENQ (0x05) /// from the process. -enquiry_string: ?[]const u8 = null, +enquiry_response: ?[]const u8 = null, /// The terminal emulator internal state. This is the abstract "terminal" /// that manages input, grid updating, etc. and is renderer-agnostic. It @@ -116,7 +116,7 @@ pub const DerivedConfig = struct { term: []const u8, grapheme_width_method: configpkg.Config.GraphemeWidthMethod, abnormal_runtime_threshold_ms: u32, - enquiry_string: ?[]const u8, + enquiry_response: ?[]const u8, pub fn init( alloc_gpa: Allocator, @@ -136,7 +136,7 @@ pub const DerivedConfig = struct { .term = config.term, .grapheme_width_method = config.@"grapheme-width-method", .abnormal_runtime_threshold_ms = config.@"abnormal-command-exit-runtime", - .enquiry_string = config.@"enquiry-string", + .enquiry_response = config.@"enquiry-response", }; } @@ -216,7 +216,7 @@ pub fn init(alloc: Allocator, opts: termio.Options) !Exec { .osc_color_report_format = config.osc_color_report_format, .data = null, .abnormal_runtime_threshold_ms = opts.config.abnormal_runtime_threshold_ms, - .enquiry_string = if (opts.config.enquiry_string) |s| try alloc.dupe(u8, s) else null, + .enquiry_response = if (opts.config.enquiry_response) |s| try alloc.dupe(u8, s) else null, }; } @@ -226,7 +226,7 @@ pub fn deinit(self: *Exec) void { // Clean up our other members self.terminal.deinit(self.alloc); - if (self.enquiry_string) |s| self.alloc.free(s); + if (self.enquiry_response) |s| self.alloc.free(s); } pub fn threadEnter(self: *Exec, thread: *termio.Thread) !ThreadData { @@ -306,7 +306,8 @@ pub fn threadEnter(self: *Exec, thread: *termio.Thread) !ThreadData { .foreground_color = self.foreground_color, .background_color = self.background_color, .osc_color_report_format = self.osc_color_report_format, - .enquiry_string = if (self.enquiry_string) |s| try self.alloc.dupe(u8, s) else null, + .enquiry_response = if (self.enquiry_response) |s| try self.alloc.dupe(u8, s) else null, + .enquiry_response_lock = .{}, }, .parser = .{ @@ -441,8 +442,12 @@ pub fn changeConfig(self: *Exec, config: *DerivedConfig) !void { config.cursor_style, config.cursor_blink, ); - if (data.terminal_stream.handler.enquiry_string) |s| data.terminal_stream.handler.alloc.free(s); - data.terminal_stream.handler.enquiry_string = if (config.enquiry_string) |s| try data.terminal_stream.handler.alloc.dupe(u8, s) else null; + { + data.terminal_stream.handler.enquiry_response_lock.lock(); + defer data.terminal_stream.handler.enquiry_response_lock.unlock(); + if (data.terminal_stream.handler.enquiry_response) |s| data.terminal_stream.handler.alloc.free(s); + data.terminal_stream.handler.enquiry_response = if (config.enquiry_response) |s| try data.terminal_stream.handler.alloc.dupe(u8, s) else null; + } } // Set the image size limits @@ -1707,12 +1712,13 @@ const StreamHandler = struct { osc_color_report_format: configpkg.Config.OSCColorReportFormat, - enquiry_string: ?[]const u8 = null, + enquiry_response: ?[]const u8, + enquiry_response_lock: std.Thread.RwLock, pub fn deinit(self: *StreamHandler) void { self.apc.deinit(); self.dcs.deinit(); - if (self.enquiry_string) |s| self.alloc.free(s); + if (self.enquiry_response) |s| self.alloc.free(s); } inline fn queueRender(self: *StreamHandler) !void { @@ -2405,8 +2411,14 @@ const StreamHandler = struct { } pub fn enquiry(self: *StreamHandler) !void { - log.debug("sending equiry string '{any}''", .{self.enquiry_string orelse ""}); - self.messageWriter(.{ .write_stable = self.enquiry_string orelse "" }); + self.enquiry_response_lock.lockShared(); + defer self.enquiry_response_lock.unlock(); + log.debug("sending enquiry response={any}", .{self.enquiry_response orelse ""}); + if (self.enquiry_response) |r| { + self.messageWriter(try termio.Message.writeReq(self.alloc, r)); + } else { + self.messageWriter(.{ .write_stable = "" }); + } } pub fn scrollDown(self: *StreamHandler, count: usize) !void { From 5286a63fc5c1e37229fd86d1ee789c979547c4d1 Mon Sep 17 00:00:00 2001 From: "Jeffrey C. Ollie" Date: Tue, 9 Jan 2024 10:29:53 -0600 Subject: [PATCH 3/6] add some more documentation about enquiry --- docs/sequences/enq.md | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/docs/sequences/enq.md b/docs/sequences/enq.md index f7d3adfdb..fed8e130d 100644 --- a/docs/sequences/enq.md +++ b/docs/sequences/enq.md @@ -14,6 +14,14 @@ The answerback can be configured in the config file using the `enquiry-response` configuration setting or on the command line using the `--enquiry-response` parameter. The default is to send an empty string (`""`). +## TODO + +- Implement method for changing the answerback on-the-fly. This could be part of + a larger configuration editor or as a stand-alone method. + ## References - https://vt100.net/docs/vt100-ug/chapter3.html +- https://documentation.help/PuTTY/config-answerback.html +- https://invisible-island.net/xterm/ctlseqs/ctlseqs.html#h4-Single-character-functions:ENQ.C29 +- https://invisible-island.net/xterm/manpage/xterm.html#VT100-Widget-Resources:answerbackString From 6112d56eedf4c567ebfe1b87422d94c1de16d135 Mon Sep 17 00:00:00 2001 From: "Jeffrey C. Ollie" Date: Tue, 9 Jan 2024 10:36:59 -0600 Subject: [PATCH 4/6] add some more documentation about enquiry on iterm2 --- docs/sequences/enq.md | 2 ++ 1 file changed, 2 insertions(+) diff --git a/docs/sequences/enq.md b/docs/sequences/enq.md index fed8e130d..3c5969f58 100644 --- a/docs/sequences/enq.md +++ b/docs/sequences/enq.md @@ -25,3 +25,5 @@ parameter. The default is to send an empty string (`""`). - https://documentation.help/PuTTY/config-answerback.html - https://invisible-island.net/xterm/ctlseqs/ctlseqs.html#h4-Single-character-functions:ENQ.C29 - https://invisible-island.net/xterm/manpage/xterm.html#VT100-Widget-Resources:answerbackString +- https://iterm2.com/documentation-preferences-profiles-terminal.html +- https://iterm2.com/documentation-scripting.html From 4b6c2f86a712ea98a3b1e1fc2746d0b582b51d81 Mon Sep 17 00:00:00 2001 From: Mitchell Hashimoto Date: Wed, 10 Jan 2024 09:15:37 -0800 Subject: [PATCH 5/6] termio/exec: remove a bunch of state that is duplicated on StreamHandler A lot of the state that we put on Exec is just there to copy to StreamHandler, but we already have it in DerivedConfig. I think this whole copy copy copy is just legacy cruft since termio.Exec is one of the older parts of the source code. This rearchitects the Exec struct to act more like Surface and Renderer where it stores its derived config. This lets us avoid a few extra allocations and removes a LOT of struct member noise from termio.Exec. For pointer lifetimes, the memory allocated is now owned by DerivedConfig. When changeConfig is called, its the only time BOTH are still alive, so we can safely swap pointers and deinit without having to duplicate across threads. This is the same as renderer/surface. --- src/config/Config.zig | 2 +- src/termio/Exec.zig | 220 +++++++++++++++++------------------------- 2 files changed, 88 insertions(+), 134 deletions(-) diff --git a/src/config/Config.zig b/src/config/Config.zig index 61b3f5b6c..acb2e7c74 100644 --- a/src/config/Config.zig +++ b/src/config/Config.zig @@ -898,7 +898,7 @@ term: []const u8 = "xterm-ghostty", /// String to send when we receive ENQ (0x05) from the command that we are /// running. Defaults to "" if not set. -@"enquiry-response": ?[]const u8 = null, +@"enquiry-response": []const u8 = "", /// This is set by the CLI parser for deinit. _arena: ?ArenaAllocator = null, diff --git a/src/termio/Exec.zig b/src/termio/Exec.zig index 561eb76eb..1deace4b7 100644 --- a/src/termio/Exec.zig +++ b/src/termio/Exec.zig @@ -44,14 +44,8 @@ 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, - -/// Equiry string - the string to send back when we receive a ENQ (0x05) -/// from the process. -enquiry_response: ?[]const u8 = null, +/// The derived configuration for this termio implementation. +config: DerivedConfig, /// The terminal emulator internal state. This is the abstract "terminal" /// that manages input, grid updating, etc. and is renderer-agnostic. It @@ -74,30 +68,6 @@ surface_mailbox: apprt.surface.Mailbox, /// The cached grid size whenever a resize is called. grid_size: renderer.GridSize, -/// The default cursor style. We need to know this so that we can set -/// it when a CSI q with default is called. -default_cursor_style: terminal.Cursor.Style, -default_cursor_blink: ?bool, -default_cursor_color: ?terminal.color.RGB, - -/// Actual cursor color -cursor_color: ?terminal.color.RGB, - -/// Default foreground color as set by the config file -default_foreground_color: terminal.color.RGB, - -/// Default background color as set by the config file -default_background_color: terminal.color.RGB, - -/// Actual foreground color -foreground_color: terminal.color.RGB, - -/// Actual background color -background_color: terminal.color.RGB, - -/// The OSC 10/11 reply style. -osc_color_report_format: configpkg.Config.OSCColorReportFormat, - /// The data associated with the currently running thread. data: ?*EventData, @@ -105,6 +75,8 @@ data: ?*EventData, /// configuration. This must be exported so that we don't need to /// pass around Config pointers which makes memory management a pain. pub const DerivedConfig = struct { + arena: ArenaAllocator, + palette: terminal.color.Palette, image_storage_limit: usize, cursor_style: terminal.Cursor.Style, @@ -122,9 +94,12 @@ pub const DerivedConfig = struct { alloc_gpa: Allocator, config: *const configpkg.Config, ) !DerivedConfig { - _ = alloc_gpa; + var arena = ArenaAllocator.init(alloc_gpa); + errdefer arena.deinit(); + const alloc = arena.allocator(); return .{ + .arena = arena, .palette = config.palette.value, .image_storage_limit = config.@"image-storage-limit", .cursor_style = config.@"cursor-style", @@ -133,25 +108,21 @@ pub const DerivedConfig = struct { .foreground = config.foreground, .background = config.background, .osc_color_report_format = config.@"osc-color-report-format", - .term = config.term, + .term = try alloc.dupe(u8, config.term), .grapheme_width_method = config.@"grapheme-width-method", .abnormal_runtime_threshold_ms = config.@"abnormal-command-exit-runtime", - .enquiry_response = config.@"enquiry-response", + .enquiry_response = try alloc.dupe(u8, config.@"enquiry-response"), }; } pub fn deinit(self: *DerivedConfig) void { - _ = self; + self.arena.deinit(); } }; /// Initialize the exec implementation. This will also start the child /// process. pub fn init(alloc: Allocator, opts: termio.Options) !Exec { - // Clean up our derived config because we don't need it after this. - var config = opts.config; - defer config.deinit(); - // Create our terminal var term = try terminal.Terminal.init( alloc, @@ -194,39 +165,20 @@ pub fn init(alloc: Allocator, opts: termio.Options) !Exec { .alloc = alloc, .terminal = term, .subprocess = subprocess, + .config = opts.config, .renderer_state = opts.renderer_state, .renderer_wakeup = opts.renderer_wakeup, .renderer_mailbox = opts.renderer_mailbox, .surface_mailbox = opts.surface_mailbox, .grid_size = opts.grid_size, - .default_cursor_style = opts.config.cursor_style, - .default_cursor_blink = opts.config.cursor_blink, - .default_cursor_color = if (opts.config.cursor_color) |col| - col.toTerminalRGB() - else - null, - .cursor_color = if (opts.config.cursor_color) |col| - col.toTerminalRGB() - else - null, - .default_foreground_color = config.foreground.toTerminalRGB(), - .default_background_color = config.background.toTerminalRGB(), - .foreground_color = config.foreground.toTerminalRGB(), - .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, - .enquiry_response = if (opts.config.enquiry_response) |s| try alloc.dupe(u8, s) else null, }; } pub fn deinit(self: *Exec) void { self.subprocess.deinit(); - - // Clean up our other members self.terminal.deinit(self.alloc); - - if (self.enquiry_response) |s| self.alloc.free(s); + self.config.deinit(); } pub fn threadEnter(self: *Exec, thread: *termio.Thread) !ThreadData { @@ -292,24 +244,13 @@ pub fn threadEnter(self: *Exec, thread: *termio.Thread) !ThreadData { .data_stream = stream, .loop = &thread.loop, .terminal_stream = .{ - .handler = .{ - .alloc = self.alloc, - .ev = ev_data_ptr, - .terminal = &self.terminal, - .grid_size = &self.grid_size, - .default_cursor_style = self.default_cursor_style, - .default_cursor_blink = self.default_cursor_blink, - .default_cursor_color = self.default_cursor_color, - .cursor_color = self.cursor_color, - .default_foreground_color = self.default_foreground_color, - .default_background_color = self.default_background_color, - .foreground_color = self.foreground_color, - .background_color = self.background_color, - .osc_color_report_format = self.osc_color_report_format, - .enquiry_response = if (self.enquiry_response) |s| try self.alloc.dupe(u8, s) else null, - .enquiry_response_lock = .{}, - }, - + .handler = StreamHandler.init( + self.alloc, + ev_data_ptr, + &self.grid_size, + &self.terminal, + &self.config, + ), .parser = .{ .osc_parser = .{ // Populate the OSC parser allocator (optional) because @@ -318,7 +259,7 @@ pub fn threadEnter(self: *Exec, thread: *termio.Thread) !ThreadData { }, }, }, - .abnormal_runtime_threshold_ms = self.abnormal_runtime_threshold_ms, + .abnormal_runtime_threshold_ms = self.config.abnormal_runtime_threshold_ms, }; errdefer ev_data_ptr.deinit(self.alloc); @@ -402,7 +343,21 @@ pub fn threadExit(self: *Exec, data: ThreadData) void { /// Update the configuration. pub fn changeConfig(self: *Exec, config: *DerivedConfig) !void { - defer config.deinit(); + // The remainder of this function is modifying terminal state or + // the read thread data, all of which requires holding the renderer + // state lock. + self.renderer_state.mutex.lock(); + defer self.renderer_state.mutex.unlock(); + + // Deinit our old config. We do this in the lock because the + // stream handler may be referencing the old config (i.e. enquiry resp) + self.config.deinit(); + self.config = config.*; + + // Update our stream handler. The stream handler uses the same + // renderer mutex so this is safe to do despite being executed + // from another thread. + if (self.data) |data| data.terminal_stream.handler.changeConfig(&self.config); // Update the configuration that we know about. // @@ -424,32 +379,6 @@ pub fn changeConfig(self: *Exec, config: *DerivedConfig) !void { } } - // Update our default cursor style - self.default_cursor_style = config.cursor_style; - self.default_cursor_blink = config.cursor_blink; - self.default_cursor_color = if (config.cursor_color) |col| - col.toTerminalRGB() - else - null; - - // Update default foreground and background colors - self.default_foreground_color = config.foreground.toTerminalRGB(); - self.default_background_color = config.background.toTerminalRGB(); - - // If we have event data, then update our active stream too - if (self.data) |data| { - data.terminal_stream.handler.changeDefaultCursor( - config.cursor_style, - config.cursor_blink, - ); - { - data.terminal_stream.handler.enquiry_response_lock.lock(); - defer data.terminal_stream.handler.enquiry_response_lock.unlock(); - if (data.terminal_stream.handler.enquiry_response) |s| data.terminal_stream.handler.alloc.free(s); - data.terminal_stream.handler.enquiry_response = if (config.enquiry_response) |s| try data.terminal_stream.handler.alloc.dupe(u8, s) else null; - } - } - // Set the image size limits try self.terminal.screen.kitty_images.setLimit( self.alloc, @@ -1711,14 +1640,60 @@ const StreamHandler = struct { background_color: terminal.color.RGB, osc_color_report_format: configpkg.Config.OSCColorReportFormat, + enquiry_response: []const u8, - enquiry_response: ?[]const u8, - enquiry_response_lock: std.Thread.RwLock, + pub fn init( + alloc: Allocator, + ev: *EventData, + grid_size: *renderer.GridSize, + t: *terminal.Terminal, + config: *const DerivedConfig, + ) StreamHandler { + const default_cursor_color = if (config.cursor_color) |col| + col.toTerminalRGB() + else + null; + + return .{ + .alloc = alloc, + .ev = ev, + .grid_size = grid_size, + .terminal = t, + .osc_color_report_format = config.osc_color_report_format, + .enquiry_response = config.enquiry_response orelse "", + .default_foreground_color = config.foreground.toTerminalRGB(), + .default_background_color = config.background.toTerminalRGB(), + .default_cursor_style = config.cursor_style, + .default_cursor_blink = config.cursor_blink, + .default_cursor_color = default_cursor_color, + .cursor_color = default_cursor_color, + .foreground_color = config.foreground.toTerminalRGB(), + .background_color = config.background.toTerminalRGB(), + }; + } pub fn deinit(self: *StreamHandler) void { self.apc.deinit(); self.dcs.deinit(); - if (self.enquiry_response) |s| self.alloc.free(s); + } + + /// Change the configuration for this handler. + pub fn changeConfig(self: *StreamHandler, config: *DerivedConfig) void { + self.osc_color_report_format = config.osc_color_report_format; + self.enquiry_response = config.enquiry_response orelse ""; + self.default_foreground_color = config.foreground.toTerminalRGB(); + self.default_background_color = config.background.toTerminalRGB(); + self.default_cursor_style = config.cursor_style; + self.default_cursor_blink = config.cursor_blink; + self.default_cursor_color = if (config.cursor_color) |col| + col.toTerminalRGB() + else + null; + + // If our cursor is the default, then we update it immediately. + if (self.default_cursor) self.setCursorStyle(.default) catch |err| { + log.warn("failed to set default cursor style: {}", .{err}); + }; } inline fn queueRender(self: *StreamHandler) !void { @@ -1770,21 +1745,6 @@ const StreamHandler = struct { self.writer_messaged = true; } - pub fn changeDefaultCursor( - self: *StreamHandler, - style: terminal.Cursor.Style, - blink: ?bool, - ) void { - self.default_cursor_style = style; - self.default_cursor_blink = blink; - - // If our cursor is the default, then we update it immediately. - if (self.default_cursor) self.setCursorStyle(.default) catch |err| { - log.warn("failed to set default cursor style: {}", .{err}); - return; - }; - } - pub fn dcsHook(self: *StreamHandler, dcs: terminal.DCS) !void { self.dcs.hook(self.alloc, dcs); } @@ -2411,14 +2371,8 @@ const StreamHandler = struct { } pub fn enquiry(self: *StreamHandler) !void { - self.enquiry_response_lock.lockShared(); - defer self.enquiry_response_lock.unlock(); - log.debug("sending enquiry response={any}", .{self.enquiry_response orelse ""}); - if (self.enquiry_response) |r| { - self.messageWriter(try termio.Message.writeReq(self.alloc, r)); - } else { - self.messageWriter(.{ .write_stable = "" }); - } + log.debug("sending enquiry response={s}", .{self.enquiry_response}); + self.messageWriter(try termio.Message.writeReq(self.alloc, self.enquiry_response)); } pub fn scrollDown(self: *StreamHandler, count: usize) !void { From 60b1b0a8dea434820e4c09e5e0ec5a3e2873a577 Mon Sep 17 00:00:00 2001 From: Mitchell Hashimoto Date: Wed, 10 Jan 2024 09:26:10 -0800 Subject: [PATCH 6/6] termio/exec: assign arena last so we capture allocations --- src/termio/Exec.zig | 16 +++++++++++----- 1 file changed, 11 insertions(+), 5 deletions(-) diff --git a/src/termio/Exec.zig b/src/termio/Exec.zig index 1deace4b7..38aec2929 100644 --- a/src/termio/Exec.zig +++ b/src/termio/Exec.zig @@ -88,7 +88,7 @@ pub const DerivedConfig = struct { term: []const u8, grapheme_width_method: configpkg.Config.GraphemeWidthMethod, abnormal_runtime_threshold_ms: u32, - enquiry_response: ?[]const u8, + enquiry_response: []const u8, pub fn init( alloc_gpa: Allocator, @@ -99,7 +99,6 @@ pub const DerivedConfig = struct { const alloc = arena.allocator(); return .{ - .arena = arena, .palette = config.palette.value, .image_storage_limit = config.@"image-storage-limit", .cursor_style = config.@"cursor-style", @@ -112,6 +111,10 @@ pub const DerivedConfig = struct { .grapheme_width_method = config.@"grapheme-width-method", .abnormal_runtime_threshold_ms = config.@"abnormal-command-exit-runtime", .enquiry_response = try alloc.dupe(u8, config.@"enquiry-response"), + + // This has to be last so that we copy AFTER the arena allocations + // above happen (Zig assigns in order). + .arena = arena, }; } @@ -1639,9 +1642,12 @@ const StreamHandler = struct { foreground_color: terminal.color.RGB, background_color: terminal.color.RGB, - osc_color_report_format: configpkg.Config.OSCColorReportFormat, + /// The response to use for ENQ requests. The memory is owned by + /// whoever owns StreamHandler. enquiry_response: []const u8, + osc_color_report_format: configpkg.Config.OSCColorReportFormat, + pub fn init( alloc: Allocator, ev: *EventData, @@ -1660,7 +1666,7 @@ const StreamHandler = struct { .grid_size = grid_size, .terminal = t, .osc_color_report_format = config.osc_color_report_format, - .enquiry_response = config.enquiry_response orelse "", + .enquiry_response = config.enquiry_response, .default_foreground_color = config.foreground.toTerminalRGB(), .default_background_color = config.background.toTerminalRGB(), .default_cursor_style = config.cursor_style, @@ -1680,7 +1686,7 @@ const StreamHandler = struct { /// Change the configuration for this handler. pub fn changeConfig(self: *StreamHandler, config: *DerivedConfig) void { self.osc_color_report_format = config.osc_color_report_format; - self.enquiry_response = config.enquiry_response orelse ""; + self.enquiry_response = config.enquiry_response; self.default_foreground_color = config.foreground.toTerminalRGB(); self.default_background_color = config.background.toTerminalRGB(); self.default_cursor_style = config.cursor_style;