From 29ac9be75fdcafbd7d05c15aa9eb8490d74caa98 Mon Sep 17 00:00:00 2001 From: Gabriel Schneider Date: Wed, 26 Aug 2026 18:58:37 -0300 Subject: An edited row keeps its colours, four copies of forkShell become one, and Esc stops recentring MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ## A terminal row's ANSI colours survive being edited The loudest colour bug this editor had: one keystroke anywhere in a coloured shell row turned EVERY column of it grey. `EditAnchors` anchored a buffer line only when it was BYTE-IDENTICAL to the shell row it stood over, so a single differing byte dropped the whole row's colour projection. Worst shape is invisible: append past the pane's right edge, where the text is clipped, and the row looks the same and only its colour goes. Anchoring is byte-level now. An edit leaves the row's own bytes at both ends, and being the same bytes they keep the same colours; only what was typed has no cell under it, so only that takes none. Live, on real `fastfetch`: a 32-column blue run split into 6 + 26 around one typed character. Three defects underneath it, all found by machinery rather than by reading: * A JOIN removes a buffer line while the buffer's covered span grows, so `lines == covered` and both aligned guesses — Nth line over the Nth covered row, and the same counted from the bottom — resolved to the SAME wrong row. Every untouched row below a join went plain. Anchoring is now a streaming monotone matching: one shell-row cursor that only ever moves forward, advanced once per buffer line, linear in the buffer where the version before it was quadratic. * An EMPTY line is not evidence. Splitting a row makes one, it equals every blank row in the span, and left free to look ahead it claimed the blank row below the last output and took every coloured row in between out of reach of the lines that owned them. * Reflow under a scrolled viewport. `PageList.getTopLeft(.viewport)` returns the viewport pin verbatim, x and all, while `PageList.pin` forces x to 0 — so after a reflow remapped a tracked pin into the middle of a row, the text pass dumped row 0 from that column while the colour pass paired the fragment with the row's FIRST cells. Row 0 wore its left half's colours until the pane snapped back to live output. `bodyText` dumps from column zero now, which is also what ghostty's own renderer draws. Also here: DECSCNM (reverse video) was silently dropped whenever `tty_filter` was off, because the raw path resolved a `.none` colour by role and never consulted the mode. The test that found the first two is the one worth keeping: random editing against an ABSOLUTE oracle — every row's own text names the colour it must have — because the differential oracle it replaced was blind by construction. It skipped the edited row, which is the row the user is complaining about. ## Esc returns to a pane without moving its view Esc in body normal mode runs `Last`, "the pane you were in before this one", and that went through `focusPaneLine`, which recentred a file on the target line unconditionally. So returning to a buffer repainted the whole screen to show a line that was already on it. `focusPaneLine` takes a landing now: `.center` for the three callers going somewhere you have not been (a look target, a path a pane already holds, `@pN:LINE:COL`), `.keep` for Esc. `.keep` leaves the view alone and lets `ensureCursorVisible` — which already existed and already scrolls by the minimum into the `scroll_off` band — be the only thing that may move anything. Not `line = 0`, which `focusPaneLine` already understands as "focus and touch nothing": a background pane's view can move while you are away, because the wheel scrolls the pane under the POINTER and a resize reveals no cursor, so the recorded cursor plus a minimal nudge is what actually gets you back. Ctrl-o and Ctrl-i keep centring, and the asymmetry is structural rather than arbitrary: `Last` only ever CROSSES panes, so the pane it lands on already holds the view you left it with, while `jumpBy` can land in the SAME pane, where a long in-file jump would arrive on the very top or bottom row with `scroll_off` lines of context on one side. Helix splits the same pair the same way — its jumplist centres, its buffer switch does not. One deliberate consequence: under `.keep` a PDF's page is not restored AT ALL, because a page reveal IS that pane's view and a reveal of the page you are already on still snaps `document_scroll_y` to that page's start, discarding where you had read to. When something moved the pane while you were away — the wheel again — Esc leaves it where the wheel left it, and Ctrl-o is how you reach the recorded page. ## host_io.zig: the machine-local half of a host, once `host.zig` is the seam. The part of the answer that is identical on every host with an operating system under it — fork a pane's shell, put bytes on a disk — was written FOUR times: in tty.zig, gui.zig, macos.zig and detached/server.zig. What those copies had in common says what they were for: all four were missing FD_CLOEXEC on the pty master, so in every shell pardes has shipped, a program in one pane could read another pane's terminal. One copy now, and the wire got smaller for it: `ServerMsg.spawn` is gone. A frontend never asked the server to fork anything — the server has an operating system under it and forks through `host_io` like every other host — and `decodeClient` lost the scratch buffer that message needed. --- src/host_io.zig | 178 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 178 insertions(+) create mode 100644 src/host_io.zig (limited to 'src/host_io.zig') diff --git a/src/host_io.zig b/src/host_io.zig new file mode 100644 index 00000000..001446b1 --- /dev/null +++ b/src/host_io.zig @@ -0,0 +1,178 @@ +//! THE MACHINE-LOCAL HALF OF A HOST: fork a pane's shell, put bytes on a disk. +//! +//! `host.zig` is the seam — the struct of function pointers the core asks +//! through. This file is the part of the answer that is the same on every host +//! that has an operating system under it, and it is now the ONLY copy of it: +//! tty.zig, detached/server.zig, gui/gui.zig and macos.zig all fork and write +//! through here. They did not always. Each of the four grew its own `forkShell` +//! and its own `writeFd`, and what those four copies were for is best said by +//! what they had in common: ALL FOUR were missing FD_CLOEXEC on the pty master, +//! so in every shell pardes has ever shipped a program in one pane could read +//! and write another pane's terminal, and closing a master did not reliably hang +//! its shell up. One line below fixes that for all four at once (see `forkShell`) +//! — which is a better argument for this file existing than "it is shared" is. +//! +//! Why the daemon and not the frontend does this work: a unix socket means the +//! core and its frontends are on the SAME machine, so there is no question of +//! whose disk or whose process table is meant. Given that, the pane shells +//! belong to the long-lived process, because the whole promise of a detached +//! session is that it outlives the frontend attached to it — a shell forked by +//! a frontend dies with that frontend, and then the session has a pane with no +//! shell in it. The frontend keeps exactly what needs the human's screen: the +//! grid, the keyboard, the clipboard and a link to open. +//! +//! So `forkShell` takes the core it is forking on behalf of and nothing about +//! terminals: no vaxis, no `Loop`, no reader thread. Who drains the master fd +//! is the caller's business, and the callers answer differently on purpose. The +//! tty, gui and macOS shells hand it to a worker that posts into their event +//! loop; the daemon adds it to the one `poll(2)` it already runs over its +//! clients, and makes its own copy non-blocking in order to. That last is why +//! `Child.file.flags` is left saying what it says: the flag describes the +//! descriptor `forkpty` handed back, for the three callers that stream it, and +//! the one that polls it keeps only the handle. +const std = @import("std"); +const posix = std.posix; +const libc = std.c; +const pardes = @import("pardes.zig"); +const shell_bin = @import("shell_bin.zig"); +const fs_service = @import("fs_service.zig"); +const fuse = @import("fuse.zig"); + +/// `setCloexec` and nothing else. Imported rather than copied a fourth time — +/// fuse.zig and nested.zig each grew a private two-line version of it — because +/// the descriptor this file has to protect is the one every OTHER file in the +/// tree already protects, and one predicate is how the reasoning stays in one +/// place. nested.zig is a leaf (std, builtin, libc), so this costs no +/// dependency worth the name. +const nested = @import("nested.zig"); + +extern "c" fn forkpty(amaster: *c_int, name: ?[*:0]u8, termp: ?*const anyopaque, winp: ?*const posix.winsize) c_int; +extern "c" fn execv(path: [*:0]const u8, argv: [*:null]const ?[*:0]const u8) c_int; +extern "c" fn chdir(path: [*:0]const u8) c_int; +extern "c" fn _exit(status: c_int) noreturn; + +/// A forked pane shell: the pty master to read and write, and the pid to reap. +/// Named rather than anonymous because four files now hold one of these. +pub const Child = struct { + file: std.Io.File, + pid: posix.pid_t, +}; + +/// Fork a shell onto a fresh pty for `pane`, sized `rows`x`cols`. +/// +/// `core` is optional because a host may fork before it has one, and a core +/// that is absent simply does not name its shell. +pub fn forkShell( + core: ?*pardes.Pardes, + pane: usize, + prompt_rcs: *const shell_bin.PromptRcs, + bin: []const u8, + cwd: ?[*:0]const u8, + rows: u16, + cols: u16, + fs: ?*const fuse.Fs, +) Child { + var master: c_int = undefined; + // resolved BEFORE the fork, into this frame, which the child inherits: + // nothing between fork and exec may allocate, and a PATH search would + var path_buf: [std.fs.max_path_bytes]u8 = undefined; + const spawn = shell_bin.resolve(bin, &path_buf, prompt_rcs); + // ...and so is the pane's own address on the control filesystem, for a + // second reason on top of that one: acme puts `winid` in the child, which + // is safe there only because rfork(RFENVG) has just given it a private + // environment group. See fs_service.exportPaneEnv. + fs_service.exportPaneEnv(fs, if (core) |c| (if (c.panes[pane]) |pn| pn.serial else 0) else 0); + const ws = posix.winsize{ .row = rows, .col = cols, .xpixel = 0, .ypixel = 0 }; + const pid = forkpty(&master, null, null, &ws); + if (pid == 0) { + // the blocked-SIGWINCH mask survives fork AND exec — unblock it or + // bash/vim in the pane would never see resizes (sigprocmask is + // async-signal-safe) + var set = posix.sigemptyset(); + posix.sigaddset(&set, posix.SIG.WINCH); + posix.sigprocmask(posix.SIG.UNBLOCK, &set, null); + if (cwd) |c| _ = chdir(c); + _ = execv(spawn.path, &spawn.argv); + _exit(127); + } + if (pid > 0) { + // CLOEXEC ON THE MASTER, and it belongs here rather than at either + // caller because `forkpty` is what opens it: /dev/ptmx is opened with no + // O_CLOEXEC and there is no flag argument to ask for one. Without this, + // every pane shell forked AFTER this one inherits this master and keeps + // it across `execv`, which is two bugs at once. + // + // The loud one: a program running in pane 3 can read pane 0's output and + // write bytes into pane 0's screen. + // + // The silent one, and the reason it compounds: closing a master is the + // only thing that hangs its shell up, and a master a later shell still + // holds open is not closed. detached/server.zig `closePty` and tty.zig + // `spawn` both depend on that hangup, so a pane delete or a respawn left + // an orphaned shell that never exits — never reaped, eventually blocked + // writing into a pty nobody reads — and each orphan pinned every earlier + // pane's master in turn. The startup drain forks pane 0 and then pane 1, + // so the arrangement existed from boot, and it existed in all four + // copies of this function before they became this one. nested.zig and + // fuse.zig say the same thing about their own descriptors ("pane shells + // are forked with forkpty and inherit everything open"); the master was + // the one descriptor in the tree that nobody had said it to. + // + // THE WINDOW THIS LEAVES, stated rather than papered over: fcntl after + // fork is not atomic, so a thread that forks and execs between these two + // syscalls inherits the master anyway. In the detached daemon there is no + // such thread — it is single-threaded by construction, which is what + // putting the pty masters in its own `poll(2)` bought. The shells with + // worker threads that can exec — tty.zig's pipe tasks above all — have a + // window two syscalls wide, and closing it means replacing `forkpty` with + // our own `posix_openpt(O_CLOEXEC)` / `grantpt` / `unlockpt` / fork / + // `setsid`, which is a different change to a different file. + nested.setCloexec(master); + if (core) |c| c.acknowledgeShell(pane, std.mem.span(spawn.path), spawn.argv[1] != null); + } + return .{ .file = .{ .handle = master, .flags = .{ .nonblocking = false } }, .pid = pid }; +} + +/// Truncate-or-create `path` and put `bytes` there. False on any failure, and +/// the caller reports it: a save that did not happen must not be announced as +/// one. +pub fn writeFileBytes(path: []const u8, bytes: []const u8) bool { + var pathbuf: [4096:0]u8 = undefined; + if (path.len >= pathbuf.len) return false; + @memcpy(pathbuf[0..path.len], path); + pathbuf[path.len] = 0; + const fd = libc.open(pathbuf[0..path.len :0], .{ .ACCMODE = .WRONLY, .CREAT = true, .TRUNC = true }, @as(libc.mode_t, 0o644)); + if (fd < 0) return false; + writeFd(fd, bytes); + _ = libc.close(fd); + return true; +} + +/// A whole-buffer write that finishes short writes, retries EINTR, and refuses +/// to loop on no progress. +/// +/// The zero guard is not bookkeeping: without it a `write(2)` that returns 0 for +/// a nonzero count is an infinite SPIN, because 0 is neither an error nor +/// progress and `off` never moves. macos.zig's copy carried the guard and its +/// reason all along — "a zero-byte write makes no progress; looping on it would +/// spin the main thread forever" — and the tty copy this file was extracted +/// from did not, so the extraction briefly promoted the weakest of the three to +/// being the shared one. All three are now this one: gui.zig and macos.zig were +/// migrated onto it, so the guard is no longer missing anywhere. +/// +/// A spin is strictly worse than the block it replaces, which is why this +/// matters more now that detached/server.zig reaches this file from a +/// single-threaded poll loop: a blocked `write` is one syscall a signal can +/// interrupt, and a spin is 100% of a core with the whole session behind it. +pub fn writeFd(fd: c_int, data: []const u8) void { + var off: usize = 0; + while (off < data.len) { + const n = libc.write(fd, data[off..].ptr, data.len - off); + if (n < 0) { + if (libc.errno(n) == .INTR) continue; + return; + } + if (n == 0) return; + off += @intCast(n); + } +} -- cgit v1.3