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. --- docs/lsp.md | 148 ++++++++++++++++++++++++++++++++++++++++++++++++++---------- 1 file changed, 124 insertions(+), 24 deletions(-) (limited to 'docs/lsp.md') diff --git a/docs/lsp.md b/docs/lsp.md index 54ffd764..1adb397b 100644 --- a/docs/lsp.md +++ b/docs/lsp.md @@ -29,18 +29,62 @@ core shell worker ``` The shell already ran this exact pattern for pty readers, so the async part is -about thirty lines per shell: `tty.zig` uses `io.concurrent` + the vaxis loop -queue, `gui.zig` uses a detached thread + the mutex queue it already had. The -web shell compiles in no backend at all (`zls_backend` is off for wasm), which -makes `lsp.supports` empty, which makes `lspRequest` return before it emits — -so on the web the effect is never even raised. `web.zig` leaves the host's -`lsp` method null and exports nothing for a response; a host that links a -backend would add both. - -Three rules make it safe: +about thirty lines per shell: `src/tty/tty.zig` uses `io.concurrent` + the +vaxis loop queue, `src/gui/gui.zig` uses a detached thread (`lspThread`) + the +mutex queue it already had. A FREESTANDING core compiles in no backend at all +(`zls_backend = !freestanding_core` in `build.zig`), which is both the web +shell and the ESP32-P4 object: there `lsp.supports` is empty, which makes +`lspRequest` return before it emits, so the effect is never even raised. +`web.zig` leaves the host's `pull_lsp` null and exports nothing for a +response; a host that links a backend would add both. + +**In a DETACHED session every query answers EMPTY.** +`src/detached/server.zig`'s vtable implements sixteen of `host.zig`'s +twenty-one methods, and `pull_lsp` is one of the five it leaves null — a +worker pool is precisely what its deliberately single-threaded loop does not +have. A null method is NOT automatically a dropped effect: `perform` decides +that per arm, and the `.lsp` arm's answer is to synthesise one on the spot — +an `lsp_resp` Event with `rows = ""`, fed straight back into `update`. `.pipe` +one arm below does the same, yielding +`pipe_resp{ .success = false, .outputs = &.{} }`, so a null `pull_pipe` is a +pipe REPORTED as failed rather than one that hangs. So `gd` jumps nowhere, +`gr` finds no references, `SPC l r` renames nothing (an empty edit list parses +as none) and Tab after a dot offers nothing — all of it indistinguishable from +a backend that found nothing, which is exactly what the seam's "no rows is a +legal answer" rule promises. + +**Tab still indents — still, not always.** The empty response reaches +`lspResponse`, whose `rows.len == 0` prong performs the indent the Tab prong +skipped, but only while the cursor has not moved. That +guard holds on the ordinary path because of `pump`'s order: `pull_wait_input`, +then the queued events, then the effects, then render. The effect Tab emitted +is performed after every keystroke that was ALREADY readable in the same +round, since `pull_wait_input` applies a whole batch and not one event (the +tty shell says so at the head of `waitInput` — "block for one event, then +apply the whole pending batch" — and the daemon's poll loop drains every +readable client `.event` straight into `core.update`). One keystroke per wake +is the normal case and the indent lands in the same frame, before render. In a +BURST where the key after Tab was readable in that same poll round, `cur_col` +has moved by the time the prong runs, the guard fails, and the Tab really is +eaten. The local shells have the same race over a wider window, so this is a +property of the late-indent repair rather than of detaching. + +None of the detached core's five null methods silently drops a reachable +effect. `pull_lsp` and +`pull_pipe` have the fallbacks above; `pull_gpio_toggle` is +`orelse return Error.NoPads` (`board_memory.zig`), which lands on the message +row; `push_post_present` is a `pump` hook fired after presenting, and there is +nothing to notify in a process with no screen; and `push_fs_reply` is the one +`perform` arm with no fallback at all, but it is only ever emitted in answer to +an `Event.fs_req`, which is not on the wire (`wire.zig`: it has no `ClientTag`, +because neither half of that pair may cross an attachment) and which the +daemon raises none of, mounting no `/dev/fuse` by its own vtable comment. + +Four rules make it safe: - **The worker never touches the core.** Path, source, arg and root are copied - into an `LspJob` before it starts (`tty.zig`). The user keeps typing while a + into an `LspJob` before it starts — one per shell, in `src/tty/tty.zig` and + `src/gui/gui.zig`. The user keeps typing while a query is in flight; a borrowed slice would be a use-after-free the length of one keystroke. - **One query in flight, identified by a monotonic id.** A second press bumps @@ -107,9 +151,11 @@ to end — the enum one level up (absolute row) and one level down (`inner/tint. a stripped path that still has a separator in it), each with the `n` step that selects it and the Enter that opens it, plus a right click. -This is the rule `look.grep` already follows for its own rows (`look.zig`, the -`shown` computation), written a second time; the two are now the same function -and want to become one. +This is the rule `look.grep` already follows for its own rows, and it is +spelled TWICE: `lsp.rel` here, and an inline `if` over the asking pane's +directory in `look.zig`'s `grep` (the `shown` computation) there. Same rule, +two implementations — they want to become one function, and `lsp.zig`'s own +comment on `rel` says so. `completion` is the kind this shape changes the most. Every other editor answers a dot with a popup of NAMES to insert; a seam that returns locations cannot @@ -166,17 +212,29 @@ asking would stop the replay dead and collapse the multicursor. ### What it costs, and what it cannot do -Per press, measured by `zig build lspbench` on this repo: +Per press. **Both timings date from 2026-08-09**, change `lmlltvrx`, and have +not been re-measured; the line count beside the first was refreshed once +afterwards, on 2026-08-12 in change `vwtlskzr`, and `src/pardes.zig` is 16 466 +lines today. The ReleaseFast column is `zig build lspbench`, which is pinned to +ReleaseFast in `build.zig` and always has been — so the Debug column came from +running the installed editor by hand and the repo records no harness for it. A +completion parses the buffer once per placeholder spelling it tries, so the +first row scales with the file: re-run rather than trusting either number. | | ReleaseFast | Debug (what `zig build` installs) | |---|---|---| | a switch arm in `src/pardes.zig` (14.6k lines) | 8.8 ms | 87 ms | | `std.` — 91 candidates, each alias-resolved into the stdlib | 26 ms | 204 ms | -It is a worker thread, so the editor does not block; but the second press of -Tab joins the first query on the UI thread (`old.cancel(io)` in the shell) and -that wait is real. Pre-existing and shared by every LSP kind — not this -feature's to fix, but it is what a fast double-Tab feels like. +It is a worker thread, so the editor does not block. On the TTY shell the +second press of Tab then joins the first query on the UI thread — +`old.cancel(s.io)` on the one in-flight future, and a backend that ignores +cancellation means waiting out a query the user already abandoned +(`src/tty/tty.zig`, the `lsp` vtable entry, whose own comment says so). The +GUI shell does not join: it spawns another thread per request and lets the +core's monotonic id make the older answer stale, so it pays memory instead of +latency. Pre-existing and shared by every LSP kind — not this feature's to +fix, but it is what a fast double-Tab feels like on a terminal. Known limitations, in the order you will meet them: @@ -194,6 +252,46 @@ Known limitations, in the order you will meet them: - **`error.`** is not handled — the position context is `.error_access`, which no branch claims. +## Which ZLS, and which stdlib + +Both are decided at build time, and `SPC l i` prints both. + +`build.zig.zon` pins ZLS to a COMMIT rather than a tag — +`git+https://github.com/zigtools/zls#3e0d082084be43e36865136a138c1fe2023b33ca`, +on the 0.16.x branch — because master requires Zig 0.17-dev and no tagged +release both builds on 0.16 and exports the internals this backend calls. +`build.zig` spells the same commit a second time, as the top-level +`const zls_version = "0.16.1-dev+3e0d0820"`, and hands it to the ZLS package's +own `-Dversion-string` and to `pardes_config.zls_version`. The duplication is +unavoidable rather than sloppy — the semver half (`0.16.1-dev`) exists nowhere +in the manifest — and it is load-bearing, because ZLS's build otherwise +derives that string from `git describe`, which has nothing to read in a +fetched package with no `.git`. The two are made to AGREE BY CONSTRUCTION: a +`comptime` block right below the constant takes the short hash after the `+`, +takes the pinned commit after the `#` in `zon.dependencies.zls.url`, and +`@compileError`s unless the first is a prefix of the second. A `.zon` bump +that forgets `build.zig` is therefore a build error, not a `SPC l i` naming a +build nobody linked. + +`std` is the harder half. ZLS resolves `@import("std")` through `zig_lib_dir` +and through nothing else, and this backend sets `zig_exe_path = null` on +purpose — asking the `zig` binary is the subprocess the whole design exists to +avoid. So `build.zig` bakes `b.graph.zig_lib_directory.path` into +`pardes_config.zig_lib_dir`: the exact stdlib pardes itself was compiled +against, which is what makes `gd` on `std.mem.count` land in the real +`mem.zig`. `zigLibPath()` (`src/lsp/lsp_zls.zig`) reads `ZIG_LIB_DIR` from the +environment FIRST and falls back to the baked path, so a user who moved the +toolchain can point the backend at it without rebuilding. With no lib dir at +all every `std` symbol is a silent miss — which is why `SPC l i` reports +whether the directory OPENS rather than only which one was compiled in. + +That same `zig_exe_path = null` is the dependency-module limitation above. +ZLS resolves a relative `.zig` path from the filesystem and `std` from the lib +dir, but any other import name — every dependency in `build.zig.zon` — it can +only answer by running `zig build --build-runner` to discover the module +graph. With no zig binary that branch returns nothing, so `@import("vaxis")` +is a silent miss by construction rather than by omission. + ## The keymap is helix's, exactly Verified against `helix-term/src/keymap/default.rs`, not from memory. @@ -299,12 +397,14 @@ hand it. `zig build lspbench` — same harness, same corpus, same 22 probes, every backend. The corpus is pardes's own `src/`, plus `test/lspfixture/`: five of the probes -point at fixtures rather than at real source, because on clean, already -formatted code the correct answer to `diagnostics` and `format` is nothing, and -that is indistinguishable from a backend that has neither. `broken.zig` carries -an unused local and a misformatted fn; `dotcomplete.zig` and `dothalf.zig` -carry the two shapes of half-typed dot. The 22 probes cover 15 of the 17 -`lsp.Kind`s +point at fixtures rather than at real source. Three of them do because on +clean, already formatted code the correct answer to `diagnostics`, +`workspace_diagnostics` and `format` is nothing, and that is indistinguishable +from a backend that has neither — `broken.zig` carries an unused local and a +misformatted fn, so all three have real work. The other two are the half-typed +dot, whose two shapes are `dotcomplete.zig` (a switch prong that parses +everywhere but at the dot) and `dothalf.zig` (a line also missing its +terminator). The 22 probes cover 15 of the 17 `lsp.Kind`s — `definition` three times, `document_symbols` twice, `completion` five times, and the two introspection kinds (`status`, `explain`) not at all. -- cgit v1.3