diff options
| author | Gabriel Schneider <[email protected]> | 2026-07-31 10:37:19 -0300 |
|---|---|---|
| committer | Gabriel Schneider <[email protected]> | 2026-08-01 15:02:08 -0300 |
| commit | 0b7a480bef38b209741c520e2478d178767a9e51 (patch) | |
| tree | 837b2222db111743cdbfc7f9706323009c330c2a | |
| parent | 5cf16eab0a4beec196e51bdcee731b899a2af07c (diff) | |
| download | pardes-0b7a480bef38b209741c520e2478d178767a9e51.tar.gz pardes-0b7a480bef38b209741c520e2478d178767a9e51.zip | |
the gui shell watches files too
It never did — the effect arm was `.watch => {}` with a comment saying a shell
that never delivers the event simply never reloads. That was written as a
deliberate scope cut and it is the whole bug: the user runs pardes-gui.
The tty watcher was fine, and I proved all four suspicions false against a real
binary in a real pty with the write done by a stranger process and NO input
delivered afterwards: the loop does wake (postEvent signals an empty queue),
zig fmt's rename-over does fire MOVED_TO and is caught, three panes across two
directories all reload and closing one leaves its neighbour still following,
and the self-write hash guard does not swallow a real change. The live session
even had its inotify mark on src with the right mask and re-read config.zig
when I touched that directory.
Duplicated rather than shared with tty.zig, the way the two shells already each
own LspJob, forkShell and their pty readers. The headless PARDES_TEST_GRID path
deliberately passes -1: its contract is one frame per scripted input, and a
reload on its own clock would put an unasked-for frame in the stream.
filewatch.snap proved less than it looked. Its real blind spots were the
rename-over shape — the old script only truncated, so it saw CLOSE_WRITE and
never MOVED_TO — multiple directories, and the one that mattered: the suite
only ever runs the TTY binary, so it structurally cannot see a gui-only
regression. It now uses a new `run` directive whose writer is a child of the
RUNNER, so nothing it does reaches an app pty.
| -rw-r--r-- | src/gui/gui.zig | 148 | ||||
| -rw-r--r-- | test/snapshot.zig | 10 | ||||
| -rw-r--r-- | test/snapshots/filewatch.golden | 73 | ||||
| -rw-r--r-- | test/snapshots/filewatch.snap | 43 |
4 files changed, 229 insertions, 45 deletions
diff --git a/src/gui/gui.zig b/src/gui/gui.zig index 998573b5..8aaeb4ec 100644 --- a/src/gui/gui.zig +++ b/src/gui/gui.zig @@ -14,6 +14,7 @@ const std = @import("std"); const builtin = @import("builtin"); const posix = std.posix; const libc = std.c; +const linux = std.os.linux; // inotify constants; referenced only on linux const vaxis = @import("vaxis"); // test modes only: the stdin escape-seq parser const ghostty_vt = @import("ghostty-vt"); // 256-color palette for .index cells const pardes = @import("../pardes.zig"); @@ -499,8 +500,18 @@ const Msg = union(enum) { eof: struct { pane: u8, gen: u32, fd: c_int }, /// a language query finished on its own thread (see lspThread) lsp: struct { id: u32, rows: []u8 }, + /// something happened in a watched directory (see watchThread) + files_changed, }; +/// The files on open panes, watched through ONE inotify instance. Same shape +/// and same reasoning as tty.zig's Watch, which spells it out: the mark goes on +/// the containing DIRECTORY because nothing rewrites a file in place — a +/// formatter, a checkout, an editor all rename a temp file over the target and +/// swap the inode — and `hash` is what we last saw ON DISK, so our own Save +/// never reads as an external change. +const Watch = struct { wd: c_int, hash: u64 }; + /// One language query, owned by the thread running it — the gui twin of /// tty.zig's LspJob, and copied for the same reason: the core edits on. const LspJob = struct { @@ -541,7 +552,7 @@ const Queue = struct { switch (m) { .output => |o| q.gpa.free(o.bytes), .lsp => |l| q.gpa.free(l.rows), - .eof => {}, + .eof, .files_changed => {}, } return; } @@ -550,7 +561,7 @@ const Queue = struct { switch (m) { .output => |o| q.gpa.free(o.bytes), .lsp => |l| q.gpa.free(l.rows), - .eof => {}, + .eof, .files_changed => {}, } return; }; @@ -578,7 +589,7 @@ const Queue = struct { for (q.items.items) |m| switch (m) { .output => |o| q.gpa.free(o.bytes), .lsp => |l| q.gpa.free(l.rows), - .eof => {}, + .eof, .files_changed => {}, }; q.items.deinit(q.gpa); } @@ -604,6 +615,79 @@ fn spawnReader(gpa: std.mem.Allocator, pt: Pty, pane: u8, gen: u32, q: *Queue) v th.detach(); } +/// Mark or unmark one pane's file (`path` null = unmark). Linux only: anywhere +/// else this returns silently, the core never receives a file_changed event, +/// and the feature is simply off — which the core already has to tolerate, +/// since the browser build of this same shell has no filesystem at all. +/// ponytail: darwin wants the FSEvents half of std.Build.Watch here. +fn watchPane(fd: c_int, watches: *[pardes.MAX_PANES]?Watch, id: u8, path: ?[]const u8, hash: u64) void { + if (comptime builtin.os.tag != .linux) return; + if (fd < 0) return; + if (watches[id]) |old| { + // inotify hands out ONE descriptor per directory, so two panes on + // files in the same directory share it: drop the mark only when the + // last of them lets go, or closing one blinds the other. + var shared = false; + for (watches, 0..) |other, i| { + const o = other orelse continue; + if (i != id and o.wd == old.wd) shared = true; + } + if (!shared) _ = libc.inotify_rm_watch(fd, old.wd); + watches[id] = null; + } + const p = path orelse return; + const dir = std.fs.path.dirname(p) orelse "."; + var dbuf: [4096:0]u8 = undefined; + if (dir.len >= dbuf.len) return; + @memcpy(dbuf[0..dir.len], dir); + dbuf[dir.len] = 0; + // CLOSE_WRITE, not MODIFY: one event when a writer is DONE rather than one + // per write(2). MOVED_TO and CREATE catch the rename-over and the + // delete-then-recreate that are how files are actually replaced. + const mask = linux.IN.CLOSE_WRITE | linux.IN.MOVED_TO | linux.IN.CREATE | linux.IN.ONLYDIR; + const wd = libc.inotify_add_watch(fd, dbuf[0..dir.len :0], mask); + if (wd < 0) return; + watches[id] = .{ .wd = wd, .hash = hash }; +} + +/// Block on the inotify fd and wake the loop. Deliberately does NOT parse the +/// events: the loop re-reads every watched pane anyway, so the only thing an +/// event carries that we need is THAT something happened, and parsing would +/// mean sharing the watch table with the thread that mutates it. Detached like +/// the pty readers, and ended the same way — teardown closes the fd, the read +/// fails, the thread returns. +fn watchThread(fd: c_int, q: *Queue) void { + var buf: [4096]u8 = undefined; + while (true) { + const n = libc.read(fd, &buf, buf.len); + if (n < 0) { + if (libc.errno(n) == .INTR) continue; + break; + } + if (n == 0) break; + q.push(.files_changed); + } +} + +/// Hand the core every watched pane whose bytes moved on disk. The wake says +/// only THAT something happened, so this re-reads the lot; the hash comparison +/// is what keeps our own Save — and any write that lands on identical content +/// — out of the undo stack. Read on the loop rather than on the watcher thread +/// because the core is the only thing that knows which pane a path belongs to. +fn reloadChanged(core: *pardes.Pardes, gpa: std.mem.Allocator, watches: *[pardes.MAX_PANES]?Watch) void { + for (watches, 0..) |*slot, id| { + if (slot.* == null) continue; + const pane = core.panes[id] orelse continue; + const f = pane.file orelse continue; + const bytes = look.readFile(gpa, f.path) catch continue; + defer gpa.free(bytes); + const h = std.hash.Wyhash.hash(0, bytes); + if (h == slot.*.?.hash) continue; + slot.*.?.hash = h; + core.update(.{ .file_changed = .{ .pane = @intCast(id), .bytes = bytes } }); + } +} + /// Answer a language query off the render loop and push the rows to the queue /// — the async execution model, spelled in the plumbing this shell already has /// (a detached thread and the mutex queue the pty readers use). @@ -973,11 +1057,25 @@ fn runNative(init: std.process.Init, opts_in: pardes.Options) !void { }; var queue: Queue = .{ .gpa = gpa, .sdl_wake = true }; defer queue.close(); + // One inotify instance for every watched pane, opened here — before any + // thread exists — so the pre-loop drain below can already mark the file a + // positional path argument opened. -1 off linux: watchPane goes quiet and + // the core simply never gets a file_changed event. + var inotify_fd: c_int = if (builtin.os.tag == .linux) libc.inotify_init1(linux.IN.CLOEXEC) else -1; + defer if (inotify_fd >= 0) { + _ = libc.close(inotify_fd); // ends the detached watcher's read + inotify_fd = -1; + }; + var watches: [pardes.MAX_PANES]?Watch = @splat(null); // initial spawns BEFORE any worker thread exists: forkpty from a // multithreaded process can wedge the child before exec (see tty.zig). - drainEffects(core, &ptys, &gens, gpa, &queue, &g, false); + drainEffects(core, &ptys, &gens, gpa, &queue, &g, inotify_fd, &watches, false); for (&ptys, 0..) |*slot, id| if (slot.*) |pt| spawnReader(gpa, pt, @intCast(id), gens[id], &queue); + // ...and the one file watcher. Started even with nothing marked yet: the fd + // already exists and an unwatched inotify instance just parks in read(2) — + // one thread for the process, however many panes come and go. + if (inotify_fd >= 0) if (std.Thread.spawn(.{}, watchThread, .{ inotify_fd, &queue })) |th| th.detach() else |_| {}; _ = c.SDL_StartTextInput(window); @@ -1001,6 +1099,7 @@ fn runNative(init: std.process.Init, opts_in: pardes.Options) !void { } // 2. pty output from the reader threads var msgs = queue.take(); + var check_files = false; for (msgs.items) |m| switch (m) { .output => |o| { if (gens[o.pane] == o.gen) @@ -1018,12 +1117,17 @@ fn runNative(init: std.process.Init, opts_in: pardes.Options) !void { core.update(.{ .lsp_resp = .{ .id = l.id, .rows = l.rows } }); gpa.free(l.rows); }, + // Coalesced on purpose: a burst of writes (a formatter, a build, a + // `git checkout`) collapses into ONE pass below, so it cannot queue + // a reload — or an undo entry — per write. + .files_changed => check_files = true, }; msgs.deinit(gpa); + if (check_files) reloadChanged(core, gpa, &watches); // 3. steamdeck: poll gamepad axes into virtual cursor / wheel events pollGamepad(&g, core); // 4. effects - drainEffects(core, &ptys, &gens, gpa, &queue, &g, true); + drainEffects(core, &ptys, &gens, gpa, &queue, &g, inotify_fd, &watches, true); if (core.quit) break; // Restore builtin: swap in a core rebuilt from the dump; kill the live // shells (their detached readers wake on child death; gens bumped so @@ -1040,6 +1144,10 @@ fn runNative(init: std.process.Init, opts_in: pardes.Options) !void { slot.* = null; }; for (&gens) |*g2| g2.* +%= 1; + // the replay core's pane ids mean new things, and the dying core's + // `watch off` effects go into a queue nobody drains — drop the lot + // here. The new core emits its own `on`s as it builds its panes. + for (0..watches.len) |wid| watchPane(inotify_fd, &watches, @intCast(wid), null, 0); core.deinit(); core = nc; } @@ -1396,7 +1504,11 @@ fn runGrid(init: std.process.Init, opts_in: pardes.Options) !void { }; var queue: Queue = .{ .gpa = gpa, .sdl_wake = false }; defer queue.close(); - drainEffects(core, &ptys, &gens, gpa, &queue, null, false); + // no inotify here on purpose: this mode's whole contract is one frame per + // scripted input event, and a reload that arrives on its own clock would + // put a frame in the stream nothing asked for. -1 makes watchPane a no-op. + var watches: [pardes.MAX_PANES]?Watch = @splat(null); + drainEffects(core, &ptys, &gens, gpa, &queue, null, -1, &watches, false); for (&ptys, 0..) |*slot, id| if (slot.*) |pt| spawnReader(gpa, pt, @intCast(id), gens[id], &queue); setStdinRaw() catch {}; // stdin may be a pipe, not a pty — best effort @@ -1435,9 +1547,10 @@ fn runGrid(init: std.process.Init, opts_in: pardes.Options) !void { gpa.free(l.rows); n_events += 1; }, + .files_changed => {}, // unreachable: no watcher thread in this mode }; msgs.deinit(gpa); - drainEffects(core, &ptys, &gens, gpa, &queue, null, true); + drainEffects(core, &ptys, &gens, gpa, &queue, null, -1, &watches, true); pollCwds(core, &ptys); if (n_events == 0) continue; // idle tick: nothing changed, no frame _ = frame_arena.reset(.retain_capacity); @@ -2069,6 +2182,8 @@ fn drainEffects( gpa: std.mem.Allocator, queue: *Queue, g: ?*Gui, // null in grid test mode (no SDL: clipboard effects are no-ops) + inotify_fd: c_int, + watches: *[pardes.MAX_PANES]?Watch, threads_ok: bool, ) void { while (core.nextEffect()) |effect| switch (effect) { @@ -2118,6 +2233,9 @@ fn drainEffects( if (fd < 0) continue; writeFd(fd, f.content); _ = libc.close(fd); + // our own write is about to come back as a watch event: restamp + // from the bytes we just put there so it reads as "no change" + if (watches[sf.pane]) |*w| w.hash = std.hash.Wyhash.hash(0, f.content); }, .write_dump => { const out = core.dump_out orelse continue; @@ -2137,11 +2255,17 @@ fn drainEffects( _ = c.SDL_SetClipboardText(z.ptr); }, .lsp => |e| if (threads_ok) spawnLsp(core, gpa, queue, e), - // ponytail: the SDL shell does not watch files. Everything the core - // needs is already here (the effect and the file_changed event) — what - // is missing is the ~50 lines of inotify plumbing in tty.zig, and a - // shell that never delivers the event simply never reloads. - .watch => {}, + .watch => |w| { + // starting, the path and the on-disk bytes are read off the core + // (same split as save_file); stopping, the pane is already gone + var path: ?[]const u8 = null; + var hash: u64 = 0; + if (w.on) if (core.panes[w.pane]) |pane| if (pane.file) |f| { + path = f.path; + hash = std.hash.Wyhash.hash(0, f.content); + }; + watchPane(inotify_fd, watches, w.pane, path, hash); + }, .quit => {}, }; } diff --git a/test/snapshot.zig b/test/snapshot.zig index 4344654c..19b65a02 100644 --- a/test/snapshot.zig +++ b/test/snapshot.zig @@ -28,6 +28,7 @@ // file <name> <content> create file in the script's cwd (before start) // lines <name> <n> [tail] create file with n numbered lines, tail on each // dirmk <name> create a subdirectory +// run <shell...> run a command in the script's cwd (NOT in a pane) // start <rows> <cols> [arg] fork the app in a pty (optional extra CLI arg) // wait <ms> <needle...> pump until needle appears on the grid (fails hard) // settle <ms> pump for a fixed duration @@ -62,6 +63,7 @@ pub const std_options: std.Options = .{ .log_level = .err }; extern "c" fn setenv(name: [*:0]const u8, value: [*:0]const u8, overwrite: c_int) c_int; extern "c" fn execvp(file: [*:0]const u8, argv: [*:null]const ?[*:0]const u8) c_int; +extern "c" fn system(cmd: [*:0]const u8) c_int; const SNAP_BASE = "/tmp/pardes-snap"; var trace_stable = false; @@ -379,6 +381,14 @@ fn runScript(arena: std.mem.Allocator, exe_z: [:0]const u8, script_path: []const try eh.writeFile(try arena.dupeZ(u8, name), ppm.items); } else if (std.mem.eql(u8, cmd, "dirmk")) { try mkdir(arena, tok.next() orelse return error.BadScript, false); + } else if (std.mem.eql(u8, cmd, "run")) { + // A writer that is NOT a pardes pane. `file` mid-script is already + // one, but it cannot express the rename-over that is how a + // formatter actually replaces a file — and the difference is a + // different inotify event. This runs in the script's cwd, as a + // child of the RUNNER, so nothing it does reaches the app's ptys: + // whatever the app then shows, it woke up for by itself. + if (system(try arena.dupeZ(u8, tok.rest())) != 0) return error.RunFailed; } else if (std.mem.eql(u8, cmd, "start")) { const rows = try std.fmt.parseInt(u16, tok.next() orelse return error.BadScript, 10); const cols = try std.fmt.parseInt(u16, tok.next() orelse return error.BadScript, 10); diff --git a/test/snapshots/filewatch.golden b/test/snapshots/filewatch.golden index 64c43c85..71a56bba 100644 --- a/test/snapshots/filewatch.golden +++ b/test/snapshots/filewatch.golden @@ -2,8 +2,8 @@ |Kill Newcol Tutor Debug NextColor Dump Find Grep Help | NOR /tmp/pardes-snap/filewatch/cwd/w.txt Save De NOR /tmp/pardes-snap/filewatch/cwd Del NOR /tmp/pardes-snap/filewatch/cwd Del | 1 liMYEDITne 1 -| 2 line 2 w.txt w.txt -| 3 line 3 +| 2 line 2 sub w.txt sub w.txt +| 3 /tmp/pardes-snap/filewatch/cwd/sub/s.txt | 4 line 4 | 5 line 5 | 6 line 6 w.txt @@ -15,12 +15,29 @@ | 12 line 12 | 13 | -| NOR /tmp/pardes-snap/filewatch/cwd Del +| NOR /tmp/pardes-snap/filewatch/cwd/sub/s.txt Sav NOR /tmp/pardes-snap/filewatch/cwd Del +| 1 line 1 +| 2 line 2 sub w.txt +| 3 line 3 +| 4 line 4 +| 5 line 5 +| 6 line 6 +| 7 line 7 +| 8 line 8 +| 9 | -| w.txt | | | +== snap reloaded grid=150x30 cursor=15,2 +|Kill Newcol Tutor Debug NextColor Dump Find Grep Help +| NOR /tmp/pardes-snap/filewatch/cwd/w.txt Save De NOR /tmp/pardes-snap/filewatch/cwd Del NOR /tmp/pardes-snap/filewatch/cwd Del +| 1 EXTERNAL +| 2 sub w.txt sub w.txt +| +| +| +| w.txt | | | @@ -29,16 +46,29 @@ | | | -== snap reloaded grid=150x30 cursor=54,5 +| NOR /tmp/pardes-snap/filewatch/cwd/sub/s.txt Sav NOR /tmp/pardes-snap/filewatch/cwd Del +| 1 line 1 +| 2 line 2 sub w.txt +| 3 line 3 +| 4 line 4 +| 5 line 5 +| 6 line 6 +| 7 line 7 +| 8 line 8 +| 9 +| +| +| +| +== snap reloaded_sub grid=150x30 cursor=15,2 |Kill Newcol Tutor Debug NextColor Dump Find Grep Help -| NOR /tmp/pardes-snap/filewatch/cwd/w.txt Save De TTY /tmp/pardes-snap/filewatch/cwd Del NOR /tmp/pardes-snap/filewatch/cwd Del -| 1 EXTERNAL $ ls -| 2 w.txt w.txt -| $ echo EXTERN''AL > w.txt -| $ +| NOR /tmp/pardes-snap/filewatch/cwd/w.txt Save De NOR /tmp/pardes-snap/filewatch/cwd Del NOR /tmp/pardes-snap/filewatch/cwd Del +| 1 EXTERNAL +| 2 sub w.txt sub w.txt | | | +| w.txt | | | @@ -46,9 +76,10 @@ | | | -| NOR /tmp/pardes-snap/filewatch/cwd Del | -| w.txt +| NOR /tmp/pardes-snap/filewatch/cwd/sub/s.txt Sav NOR /tmp/pardes-snap/filewatch/cwd Del +| 1 RENAMED +| 2 sub w.txt | | | @@ -62,13 +93,13 @@ | == snap undone grid=150x30 cursor=15,2 |Kill Newcol Tutor Debug NextColor Dump Find Grep Help -| NOR /tmp/pardes-snap/filewatch/cwd/w.txt Save De TTY /tmp/pardes-snap/filewatch/cwd Del NOR /tmp/pardes-snap/filewatch/cwd Del -| 1 liMYEDITne 1 $ ls -| 2 line 2 w.txt w.txt -| 3 line 3 $ echo EXTERN''AL > w.txt -| 4 line 4 $ +| NOR /tmp/pardes-snap/filewatch/cwd/w.txt Save De NOR /tmp/pardes-snap/filewatch/cwd Del NOR /tmp/pardes-snap/filewatch/cwd Del +| 1 liMYEDITne 1 +| 2 line 2 sub w.txt sub w.txt +| 3 /tmp/pardes-snap/filewatch/cwd/sub/s.txt +| 4 line 4 | 5 line 5 -| 6 line 6 +| 6 line 6 w.txt | 7 line 7 | 8 line 8 | 9 line 9 @@ -77,9 +108,9 @@ | 12 line 12 | 13 | -| NOR /tmp/pardes-snap/filewatch/cwd Del -| -| w.txt +| NOR /tmp/pardes-snap/filewatch/cwd/sub/s.txt Sav NOR /tmp/pardes-snap/filewatch/cwd Del +| 1 RENAMED +| 2 sub w.txt | | | diff --git a/test/snapshots/filewatch.snap b/test/snapshots/filewatch.snap index 1ac8d081..6f1f2976 100644 --- a/test/snapshots/filewatch.snap +++ b/test/snapshots/filewatch.snap @@ -1,7 +1,23 @@ # external file updates: a write from outside pardes lands in the open pane, # and Undo brings the unsaved edit back. That is the whole contract — the # update is committed like any other edit, so nothing has to be merged. -lines w.txt 12 +# +# The writer must be a STRANGER to the editor. This script used to have a +# pardes PANE run the write, which proved much less than it looked: that +# shell's own pty output wakes the loop and paints a frame, so the reload +# could be riding on the keystroke's frame instead of on the watch. `run` is a +# child of the RUNNER — nothing it does reaches an app pty — and every +# assertion below it is a `wait`, which only READS the pty. So between the +# write and the reload the app receives no keystroke, no resize, no mouse: if +# the watch does not wake the loop by itself, these waits time out. +# +# Two files in two directories, written the two ways files actually change: +# w.txt in place (CLOSE_WRITE) and sub/s.txt by rename-over, which is what +# `zig fmt`, `git checkout` and every editor's atomic save do (MOVED_TO, and +# a new inode — the reason the mark is on the directory). +dirmk sub +file w.txt line 1\nline 2\n/tmp/pardes-snap/filewatch/cwd/sub/s.txt\nline 4\nline 5\nline 6\nline 7\nline 8\nline 9\nline 10\nline 11\nline 12\n +lines sub/s.txt 8 start 30 150 -n 3 wait 8000 w.txt stable 700 20000 @@ -15,9 +31,15 @@ key esc settle 100 press right 6 8 release right 6 8 -wait 10000 line 3 +wait 10000 line 4 stable 700 15000 -# an UNSAVED edit on line 1 +# ...and open sub/s.txt from the path sitting on line 3 of that pane, so the +# second watched pane is in a DIFFERENT directory from the first +press right 20 5 +release right 20 5 +wait 10000 line 8 +stable 700 15000 +# an UNSAVED edit on line 1 of w.txt press left 10 3 release left 10 3 stable 400 5000 @@ -27,18 +49,15 @@ key esc settle 100 stable 400 5000 snap edited -# overwrite the file from a shell in another column. The marker is split so -# the ECHOED command line never matches the wait — only the reloaded pane does -press left 60 5 -release left 60 5 -stable 400 5000 -key c-b -stable 600 8000 -text echo EXTERN''AL > w.txt -key enter +# ---- from here to `snap reloaded_sub`, pardes is sent NOTHING ---- +run printf 'EXTERNAL\n' > w.txt wait 10000 EXTERNAL stable 700 10000 snap reloaded +run printf 'RENAMED\n' > sub/t && mv sub/t sub/s.txt +wait 10000 RENAMED +stable 700 10000 +snap reloaded_sub # back in the file pane: one undo and the unsaved edit is there again press left 10 3 release left 10 3 |
