aboutsummaryrefslogtreecommitdiffstats
path: root/HANDOVER.md
blob: 3ddad3c7465d77ebb9e953a74737929fe46622d6 (plain) (blame)
1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17
18
19
20
21
22
23
24
25
26
27
28
29
30
31
32
33
34
35
36
37
38
39
40
41
42
43
44
45
46
47
48
49
50
51
52
53
54
55
56
57
58
59
60
61
62
63
64
65
66
67
68
69
70
71
72
73
74
75
76
77
78
79
80
81
82
83
84
85
86
87
88
89
90
91
92
93
94
95
96
97
98
99
100
101
102
103
104
105
106
107
108
109
110
111
112
113
114
115
116
117
118
119
120
121
122
123
124
125
126
127
128
129
130
131
132
133
134
135
136
137
138
139
140
141
142
143
144
145
146
147
148
149
150
151
152
153
154
155
156
157
158
159
160
161
162
163
164
165
166
167
168
169
170
171
172
173
174
175
176
177
178
179
180
181
182
183
184
185
186
187
188
189
190
191
192
193
194
195
196
197
198
199
200
201
202
203
204
205
206
207
208
209
210
211
212
213
214
215
216
217
218
219
220
221
222
223
224
225
226
227
228
229
230
231
232
233
234
235
236
237
238
239
240
241
242
243
244
245
246
247
248
249
250
251
252
253
254
255
256
257
258
259
260
261
262
263
264
265
266
267
268
269
270
271
272
273
274
275
276
277
278
279
280
281
282
283
284
285
286
287
288
289
290
291
292
293
294
295
296
297
298
299
300
301
302
303
304
305
306
307
308
309
310
311
312
313
314
315
316
317
318
319
320
321
322
323
324
325
326
327
328
329
330
331
332
333
334
335
336
337
338
339
340
341
342
343
344
345
346
347
348
349
350
351
352
353
354
355
356
357
358
359
360
361
362
363
364
365
366
367
368
369
370
371
372
373
374
375
376
377
378
379
380
381
382
383
384
385
386
387
388
389
390
391
392
393
394
395
396
397
398
399
400
401
402
403
404
405
406
407
408
409
410
411
412
413
414
415
416
417
418
419
420
421
422
423
424
# Handover

Written at the end of a long session, for whoever picks this up next — most
likely me, without the memory of having written any of it.

`DESIGN.md` says what this is and why. `README.md` says what it does.
`.fastfeedback/SPEED.md` says how to run things quickly. This file is the part
that is only learnable by having got it wrong.

## Where things stand

Everything is committed and pushed; `git log` is the honest changelog and the
messages are long on purpose. The working tree should be clean — if it is not,
that is unfinished work and not a stash worth trusting.

Built and tested with `make test` (~6s, parallel, per-file stamps) and
`make test-live` (~7s, needs real ptys). Both green. Clean build, no warnings.

## The rule this codebase keeps, and how I kept breaking it

**Anything derived from the layout is recomputed every frame and must not be
remembered between frames.** `rect`, `content`, `collapsed` and `hidden` all
say so in their comments. The corollary is the part that bites: *intent* is
different and does have to be stored — `weight`, `minimized`, a tab's `zoom`,
which pane has focus.

Three bugs in one afternoon came from confusing the two:

- A drag held an **index into a corner list rebuilt every frame**, by a drag
  whose whole job was changing the layout. It silently started moving other
  panes' boundaries. Fixed by storing the boundaries themselves.
- `layout_node` clears `collapsed`/`hidden` at entry. Putting `minimized` next
  to them would have un-minimised everything every frame; it lives beside
  `weight` instead, with a comment saying why.
- A test counted visible panes across *all* tabs. Only the current tab is laid
  out, so every other tab's rects are whatever they were last time it was on
  screen.

If something works and then stops working while you drag or resize, this is the
first thing to suspect.

## Traps that cost real time

**The dwell repaint list.** `app_next_deadline_ms` lists the hit-action
prefixes that arm after the pointer rests. Anything that arms on dwell and is
*not* listed there appears only when some unrelated event happens to repaint —
so it looks intermittent, which is the worst way to be wrong. This caught me
twice, for `edge:` and then `corner:`.

**Glyph width and glyph weight are different problems.** Width is now handled:
`screen_text` and `cells()` ask lib-vt's own table
(`ghostty_unicode_grapheme_width`), the same one the terminal uses for pane
content, so a two-column bell mark claims two columns and the wide cell's
right half is a tail the diff skips. Set `bell_mark "🔔"` and the title row
stays aligned. What is *not* solved is weight: a glyph that does not fill its
cell donates that space to the gap beside it and looks badly spaced next to
one that does — the cells can be provably even and still look wrong. Three
attempts at the frame buttons established this; they are plain ASCII now. If
someone reports spacing, dump the actual cells before touching any arithmetic.

Two traps found while fixing the width, both worth knowing before touching
this code again. `ghostty_unicode_grapheme_width` measures **only the first
cluster** and tells you how many codepoints it ate; a loop that ignores the
return value and instead asks "did the width change when I added the next
codepoint?" concludes that an entire ASCII string is one cluster, which
silently shrank the cheatsheet modal to ten columns. And a grapheme cluster
can be longer than the 16 bytes a cell holds (a ZWJ family emoji is 18), so
both the cell writer and the config's mark fields clamp on a codepoint
boundary — truncating mid-codepoint put invalid UTF-8 into the snapshot, which
surfaced as a decode error in a test rather than anything resembling the
cause.

**The test harness is not a session, and the difference hid a real bug.** The
server set `signal(SIGCHLD, SIG_IGN)`, which does not merely skip a handler —
it tells the kernel to discard exit statuses, so `waitpid` can never report
one. The headless driver never set it, so every test agreed that a dead pane
knew why it died, and the first real session showed `[process exited]` with no
status at all. Found by looking, in a zellij pane, thirty seconds after the
suite went green. If something is right in tests and wrong in front of you,
suspect what `server.c` does that `headless.c` does not.

**A dead pane's fd is closed at EOF, deliberately.** Panes used to be reaped
before the next paint, so nothing had to think about it. Now that they stay,
an EOF fd left in the poll set is *readable forever*: the session would spin a
core behind a pane that looks idle. `collect_cb` skips `fd < 0` for the same
reason, and that is why `sl0ppty run cmd` still terminates — the settle loop
ends when no pane has an fd left.

**`settle N` in the test harness is a sleep in disguise.** `send` and
`snapshot` are already synchronous with the app, so a settle before asserting
on *our own chrome* waits for nothing. It is only needed to pump a pane's pty.
Use `until_text()` where the answer is observable. Getting this wrong is what
made the suite 80 seconds; it is 6 now.

**Tests pinned to incidental coordinates.** Several asserted a style at a
hardcoded column, or a shape that happened to be true. They only break when
something unrelated moves, and then they look like real regressions. Assert the
thing you mean.

## Architecture in five sentences

`app.c` owns the layout tree, drawing, and input routing; `pane.c` wraps one
pty plus one libghostty-vt terminal; `screen.c` is a cell buffer that diffs
itself and emits the minimal byte stream. Everything painted registers its
`(rect, action)` into a hit list *as it is painted*, and a click is a lookup in
that list — this is D1 and it is why drawing and hit-testing cannot disagree.
Layout runs twice a frame: once as a probe to ask whether the tab fits, once
for real, and the probe must not resize panes. Shaders are a colour pass over
pane *contents* between `pane_compose` and the chrome drawn over them, which is
what makes "contents, not chrome" the paint order rather than a rule. The
control API in `cmd.c` is the same vocabulary the headless test driver speaks,
so a script written against one works against the other.

## Dead panes (D14), and what it changed

Since the first version: **only panes that were given a command keep their
corpse.** The distinguishing mark is `pane_label()`, which already held the
command a layout gave it and is empty for a plain shell -- so the rule needed
no new state, only a policy. `keep_dead` is `commands` (default), `all` or
`none`.

A pane now outlives its program: it keeps its contents, writes
`[process exited: status 3]` into its own backlog, and offers `[re-run]` and
`[close]` in its bottom frame row. Three things about it are worth knowing
before changing anything near it.

- **The dead row and the OSC 5577 row are the same row**, by choice. A dead
  pane's own buttons are inert — a click would write into a closed pty — so
  `draw_pane_status` swaps the list rather than adding a second mechanism.
  `close` is listed last so it is drawn rightmost and therefore survives a
  narrow frame; the thing that drops off is `re-run`.
- **Re-running is the same pane.** Same id, same node, same terminal, so
  nothing that referred to it needs telling and the previous run stays above
  in the scrollback. That is the whole point; do not be tempted to close and
  reopen.
- **The exit status is collected with a bounded wait, never a blocking one.**
  EOF means the program closed its fds, which for an exit already happened, so
  a few hundred microseconds wins the race with the kernel's exit path. A
  blocking `waitpid` would hang the entire session on one program that closes
  its terminal and keeps running. Losing the race costs the words "status 0".

## Shaders: what is measured, and what is not worth doing

Before optimising anything here, read the numbers rather than reasoning about
them — `.scratch/shbench.c` and `.scratch/shprof.c` produce them in a second
(they are gitignored; rebuild with the command in their headers).

What they say, per cell per pass on this box:

- the **arithmetic is free**. `vignette` — squared distances, clamp, divide —
  is *cheaper* than `dim`, which computes nothing. So precomputing an amount
  per cell, the obvious optimisation, buys approximately zero.
- the cost is per-cell overhead and the colour mixing. Removing `screen_at()`
  from the inner loop took the pass from 8.0ns to 6.1ns for ten lines, and
  that is done.
- the next real win is **per-row dispatch** (an indirect call per row rather
  than per cell, which also lets each shader hoist its row-invariant work):
  measured at 4.7ns in a prototype. Not done, because it is a rewrite of every
  shader and nothing needs the 1.4ns yet.
- an **interpreted** shader is 56ns against a compiled 8ns *per evaluation* --
  which is why D16's expressions are memoised rather than run per frame. With
  the map, a config-written positional shader is 5.2ns/cell against a compiled
  vignette's 6.1ns; only a clock-reading one pays the interpreter every frame,
  at 35ns/cell.

All of this is 0.24ms per frame on a 200x50 pane, under 3% of a 120Hz frame.
It has never been the bottleneck; do not spend a day here without a profile
saying otherwise.

## The expression memo, and the one thing wrong with it

`expr_amount_map()` caches per *program*, keyed on everything the compiler
says the program reads. Two panes of **different sizes** sharing one config
shader therefore fight over that one buffer and rebuild it each time -- which
costs what interpreting per cell would have cost, so it degrades to the slow
path rather than going wrong. Same for one focused and one unfocused pane
when the expression reads `cursor` or `focused`. The fix, if it ever matters,
is to hang the map off the pane rather than the program; the key logic does
not change, only where the buffer lives.

The thing that must not change is *what* the key contains. It is derived from
the source by the compiler (`expr_deps`), so it cannot drift from what the
program actually reads -- the same reason the layout recomputes rather than
remembers. If you add a variable to the language, add its dependency flag in
the same table, and the cache stays correct for free.

## Kitty graphics: it looked done and was not

The test suite was green and the feature did not work, because every test
transmitted *and* placed in one command (`a=T`) with explicit `c=`/`r=`. Real
programs use the other half of the protocol -- upload once with `a=t`, place
per frame with `a=p`, no cell counts -- and every part of that was broken:

- `pty_spawn`/`pty_resize` never set `ws_xpixel`/`ws_ypixel`, so a program
  asking its tty how big a cell is got zeroes and guessed;
- `ghostty_terminal_resize` was called with `0, 0` for the cell size, so
  lib-vt could not size a placement that did not say how many cells it wanted
  -- it computed zero cells, and we skipped it;
- the vendored lib freed image *data* on `ED 2`, so a full-screen program lost
  everything it had uploaded (`vendor/patches/0001`).

Three layers, one symptom: nothing on screen. Worth knowing for next time --
**the shape of the test decided what worked**. If you add a protocol here,
write the test the way a program actually uses it, not the way that is easiest
to write.

The debugging that found it is worth repeating rather than reinventing:
capture the real program's byte stream off a pty, replay it into a headless
session (it reproduces exactly), then bisect the stream. `.scratch/capfile.py`
and friends do this and are gitignored; the whole loop is about a minute.

The 8x16 default matters. Where nobody knows the cell size -- headless, or a
terminal that will not say -- a plausible cell means images appear at roughly
the right size, and a zero means they do not appear at all.

Then it rendered, and it was still wrong twice, both only visible in motion:

- we dropped the placement's **sub-cell offset** (`X=`/`Y=`), so a moving
  image snapped a whole cell at a time;
- we passed on **`c=`/`r=`** for every placement, which means *scale into this
  many cells* -- and the count a natural-size image covers goes up by one
  whenever it straddles another boundary, so it changed size as it moved.

Both were invisible in a still picture, which is exactly what the test suite
was looking at. `{"cmd":"graphics","format":"bytes"}` exists because of this:
it answers with the escape sequences the client is actually sent, so a test
can assert on the bytes rather than on the model that produced them. If you
touch graphics, assert on the bytes.

### And then a PNG arrived

Same feature, same shape of bug, a year of confidence later: kitty graphics
were green and a real program still drew nothing. lib-vt ships **no image
codec** — built as a library it sets `sys.decode_png` to null, and a null hook
rejects every `f=100` transmission outright, so no image is stored, no
placement exists, and the re-emission layer is never even reached. Raw RGB
worked, which is why it looked fine here for as long as it did: every test in
this file sent raw pixels, because that is what is easy to write in a test.
That is the *same* lesson as the section above, learned again — write the test
the way a program actually behaves. A program with a picture has a PNG.

The fix is `src/png.c`: `ghostty_sys_set(GHOSTTY_SYS_OPT_DECODE_PNG, ...)`
over the `stb_image.h` that libghostty-vt already vendors and we already
commit. See D18 for why that rather than a hand-rolled one.

The same program found the other half: it asks `CSI 16 t` for the cell size in
the same round trip as its graphics query, and we answered nothing, so it fell
back to guessing 10x20 and would have drawn the picture stretched even once
the png decoded. `GHOSTTY_TERMINAL_OPT_SIZE` answers 14/16/18 t now. If you
add anything that a program can *ask* rather than be told, check both: the
pty's pixel fields and the escape query are different code paths to the same
number, and a program that has been through an ssh hop uses the second one.

**A false lead worth not repeating.** The first bisect built its escape
sequences with `sh printf`, and `printf` mangles adjacent backslash escapes:
`\033\\` immediately followed by `\033` reaches the pty as a literal `\033`.
So the pane never received the second escape, and the evidence pointed
convincingly at "two APC sequences back to back are dropped by the parser" —
a bug in the vendored lib that did not exist. What killed it was dumping the
bytes at `pane_pump`, where the pty read shows what *actually* arrived. If a
repro's conclusion implicates the parser, check the pty bytes before you
believe it, and build test streams in python where a byte is a byte.

**Anything written after the frame must leave the cursor where the frame put
it.** Images go out after the cell diff on purpose (a repainted cell must not
land on top of a placement), and placing an image parks the cursor on its
target cell first — so the last thing the terminal heard each frame was "go
to wherever that image is". The cursor then sat in another pane, wandering
about with the picture, and it was reported as "the shell I am typing in has
no cursor". The graphics stream now opens with DECSC and closes with DECRC,
and emits nothing at all when there is nothing to say. Any future
after-the-frame output owes the same debt.

## Modals

`modal_frame()` draws the surface and `draw_scrim()` pushes back what is
behind it; the cheatsheet is the first thing to use them and is meant to be
the shape the next one copies. Two things it got wrong first, both worth not
repeating:

- **it borrowed colours from another surface.** The close button was the pane
  button colour on the finder background, which are the same value in the
  default theme, so it was invisible until hovered. A modal is its own
  surface and now has its own `modal_*` names. When you add a surface, give
  it names; when you reuse names, check what they resolve to *together*.
- **the scrim is the shader pass, aimed at the screen.** Nothing about it is
  special-cased, which is why it dims chrome as happily as contents — but it
  also means the whole screen's unset colours get materialised through
  `default_fg`/`default_bg` while a modal is up, so a terminal whose real
  background differs from the configured one shifts slightly. Documented next
  to `modal_scrim` rather than hidden.

## Leader-less bindings

`keys { direct { bind ... } }` fires without the prefix. Three ordering facts
hold it together, and all three are tested because none is obvious from the
code:

- direct lookup happens **last** in the key path, after the rename editor, the
  finder, the cheatsheet and the prefix itself. So an overlay still owns the
  keyboard, and a direct binding cannot shadow the leader — which would make
  the leader unusable with no way back.
- `direct` is part of a binding's **identity**, not a property of it: `x`
  after the leader and `x` on its own are two bindings, and binding one must
  not silently redefine the other.
- `config_lookup()` (the after-the-leader path) falls through to direct
  bindings, so pressing the prefix first never makes a binding stop working.
  A rule anybody has to keep track of is a rule that will surprise them.

The cheatsheet lists them in their own section, because the caption at the top
says "<prefix> then:" and these are exactly the ones that is not true of.

## Restarting into the same screen

`dump-layout` is the inverse of `--layout`, and `contrib/sl0ppty-dev` is the
loop that uses it: dump, quit, come back on the new binary with the same
shape. Three things it got wrong first, all of the same kind -- a dump that
loads without complaint and rebuilds something subtly different:

- **`split=` belongs on the node that has the children**, not on the children.
  Written the other way it parses fine and gives you the wrong tree.
- **a split node's weight was not being restored**, because the edit that was
  supposed to read it never applied -- a string replacement that silently
  matched nothing. The round-trip test caught it; nothing else would have.
- **`active=true` pointed at the wrong tab.** The index is recorded while the
  new tabs sit *after* the old ones, and `replace` then moves them to the
  front, so it has to shift by the number dropped. Off by that, the restore
  looks like it half-works.

The test that matters is the round trip, and specifically that a dump of a
reload is byte-identical to the first dump. Anything the dumper loses shows up
there immediately, where "does it look right" would not.

## Coverage, and what its numbers mean

`make coverage` builds a gcc `--coverage` binary into build/cov, runs every
suite against it through `$SL0PPTY_BIN`, and prints a per-file table. About a
minute, nothing installed, the zig build untouched. Currently ~85%.

Two things about the numbers, both worth knowing before anyone acts on them:

- **server.c reads ~8% and is not untested.** The session server is a
  daemonised child that ends with `_exit()`, which is correct there and skips
  the flush gcov's counters rely on. test_session.py and the live suite drive
  that code; their counts are simply lost. The report says so in its own
  output, because an unexplained low number invites someone to "fix" working
  code.
- **The C unit tests link the same instrumented objects**, so input.c and
  friends are credited properly. Building them separately made input.c read
  54% when its decoder has a dedicated unit test -- a lie about where the
  tests are.

## The config file, and the two places it lives

`sl0ppty --dump-config` renders every setting from the live values -- there is
no checked-in copy of the defaults to drift, which was the actual problem:
config/config.kdl had already lost four theme colours before anyone noticed.
That file is now explicitly the *prose* (why a setting exists), and the values
are generated.

Three things keep it honest, and the third is the one that matters:

- the theme colours are a single file-scope table that the parser *and* the
  renderer walk, so a colour cannot exist in one and not the other;
- the dump has to parse back to itself byte-for-byte, which is what caught
  `bind \ split-cols` being rendered unquoted;
- a test scans config.c for `kdl_child(root, "...")` -- the parser's own
  vocabulary -- and fails if any of those keys is missing from the dump. Add a
  knob, forget to render it, and the suite says so.

`C-a e` writes that dump when you have no config at all, directories included.
It never touches a file that exists (`fopen(..., "wx")`), which is the one
thing it must never get wrong.

## Things left on the table

- **A `reload` keybinding.** The config watcher made it less pressing.
- **Tab-level shaders**, and the bigger one: **OSC 5577 → shader**, letting a
  pane colour itself by reporting its own state. That is the idea most likely
  to make this feel unlike other multiplexers.
- **Collapsed headers do not reach the shader pass** and never will: a header
  is chrome, and shaders colour contents (D13). The gap is closed the only
  honest way, by having the header *say* what the colour says elsewhere —
  `exited: status 3`, `not started`, `▲ 12` — in the same order the status
  line ranks them. If you add a not-live state, add it in both places or a
  flattened tab will hide it.
- **A dead pane cannot be re-run from the finder or the minimised bar** — only
  from its own frame, `C-a r`, or `{"cmd":"rerun"}`. Fine while a dead pane is
  something you are looking at; less fine once you have six of them put away.
- **Nothing re-runs a whole tab**, which is the obvious next want once one
  pane can be re-run.
- **A dump does not record minimised or zoomed panes.** Both are intent and
  both would restore cleanly; they were left out because the layout parser
  has no word for either yet.
- **Shader plugins cannot be replaced without a new session** (D15): loading
  is additive on purpose, because a `shade_fn` may be live in a config or on a
  pane. Hot-swapping would need every shader reference to be indirected
  through the registry, which is a real change and not obviously worth it.
- **Per-row shader dispatch** — see the numbers above.
- **The finder is not a modal** and predates `modal_frame()`. It keeps its own
  `finder_*` colours and its own box. Moving it over would unify two overlay
  looks into one; it was left alone because it is a picker rather than a
  modal — it navigates, and modals here dismiss on any key.
- **The cell size is not queried, only received.** If a client's terminal
  reports no pixel size in `TIOCGWINSZ`, we keep the 8x16 default rather than
  asking it with `CSI 14 t`/`CSI 16 t`. Doing that properly means the client
  parsing a reply, and the client deliberately does not decode anything (D7).
  The honest fix is a config key.
- **Expressions cannot make a colour**, only a strength (D16). `tint` takes a
  fixed `color=`; a gradient *between* two colours needs either three
  expressions (one per channel) or a plugin. Worth doing only if somebody
  wants it; the one-byte result is what makes the memo a byte map.
- **Corner crossings**: found for both nestings now, but if more turn up
  missing, ask *how the layout was built* — the split order decides the tree
  shape, and that is where any remaining blind spot will be.

## How to work on it

Read the comment above the thing before changing it; they explain the decision,
not the mechanics, and several are load-bearing. Run `make test` constantly —
it is six seconds. Drive the thing in a zellij pane and look at it; every bug
in this file was found by looking, not by reasoning. Write the test that would
have caught it, and check the test fails against the old build before believing
it.