review: the two-facts claim was stated in four places, and I changed one
Self-review against the code-review skill, and it is gotcha 48's own third lesson turning up in the change that cites it: "when a fact is stated in more than one place, changing it is a grep, not an edit." `context/code/clients.md` said `#status` composes two facts in three further places -- the shell.js inventory, the transports section and the two-numbers section -- all still describing the `.` join this branch deletes. A reader arriving at any of them would have been told the opposite of what the code does. The minibuffer section also predated `#bottom-dock` and said nothing about what happens when a toast and the prompt coincide. Also drops `createToasts().showing()`, which nothing calls. It was speculative API on a single-use object (CLAUDE.md section 2), and an unexercised accessor on a timer is one more thing that can be wrong. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01FC2jVn8Au7E9JPkw6bwXXH
This commit is contained in:
+15
-8
@@ -1268,9 +1268,11 @@ Four smaller decisions that would be re-derived wrongly:
|
||||
home (gotcha 11); `None` is "no text-entry mode is open", so the page asks
|
||||
nothing about the mode. The colour stays semantic — `MinibufColor` crosses and
|
||||
the CSS class is the page's last translation, exactly as `draw.rs::minibuf_fg`
|
||||
is the TUI's (gotcha 26). It is drawn as a line of its own rather than *in
|
||||
is the TUI's (gotcha 26). It is drawn as a surface of its own rather than *in
|
||||
place of* `#status` as the TUI's bottom bar does, because `#status` also
|
||||
carries the transport's failures and a repaint must not erase one (gotcha 48).
|
||||
Since improvise-n6t8 it shares `#bottom-dock` with the toast, which stacks
|
||||
**above** it, so the prompt is neither covered nor displaced.
|
||||
|
||||
`RenderCell::edit_text` is the stored value, not the formatted one: seeding an
|
||||
editor from `text` and committing turns `1200` into the string `"1,200"`. The
|
||||
@@ -1319,9 +1321,9 @@ labels is dropped, not replayed), and the *first* frame out must be the
|
||||
viewport, because the server builds the snapshot at it.
|
||||
|
||||
Everything in this file that is a *decision* rather than a paint now lives in
|
||||
`web/shell.js` — `socketUrl`, the `#status` composition, the watchdog, the
|
||||
extent arithmetic and `gridPlan` — so that `node --test` can reach it without a
|
||||
browser. The sections below describe those rules; the code is one file over.
|
||||
`web/shell.js` — `socketUrl`, the two status sinks, the toast's identity rule,
|
||||
the watchdog, the extent arithmetic and `gridPlan` — so that `node --test` can
|
||||
reach it without a browser. The sections below describe those rules; the code is one file over.
|
||||
|
||||
### Two transports, one client
|
||||
|
||||
@@ -1345,9 +1347,13 @@ peer that never arrives reads differently from one that went away.
|
||||
socket that never opened keeps saying "cannot reach …", because the close event
|
||||
there is the tail of the failure and not news.
|
||||
|
||||
`#status` carries **two independent facts** — what the transport is doing and
|
||||
what the document last said — so it composes them rather than letting a repaint
|
||||
quietly erase a connection error.
|
||||
There are **two independent facts** here — what the transport is doing and what
|
||||
the document last said — and until improvise-n6t8 `#status` composed them with a
|
||||
`·` so that a repaint could not quietly erase a connection error. They now go to
|
||||
**two elements**: `#status` is the transport's alone and never auto-hides, and
|
||||
the document's is a toast. The rule is unchanged and the mechanism is stronger —
|
||||
a repaint reaches `onModel` and cannot address `onShell` at all. See *The bar is
|
||||
reserved, not fitted* below.
|
||||
|
||||
### `socketUrl`: `wss://` is the default
|
||||
|
||||
@@ -2182,7 +2188,8 @@ both, so the disagreement is unconstructible rather than discouraged.
|
||||
|
||||
Everything **between** those seams is `node --test web/tests`, and it may assume
|
||||
the boundary holds rather than re-establishing it: transport selection from
|
||||
`?ws=`, the two facts `#status` composes, the watchdog's arm and disarm, the
|
||||
`?ws=`, the two sinks the status facts go to, the toast's identity rule (the one
|
||||
thing a repaint-driven surface must get right), the watchdog's arm and disarm, the
|
||||
`frameCost`/extent arithmetic, the collapse glyph, the grid's plan, and the
|
||||
worker's tail-promise ordering. None of it needs a DOM, so none of it is
|
||||
Playwright's.
|
||||
|
||||
@@ -46,7 +46,6 @@ export function createToasts({
|
||||
onHide();
|
||||
}, ttlMs);
|
||||
},
|
||||
showing: () => timer !== null,
|
||||
};
|
||||
}
|
||||
|
||||
|
||||
Reference in New Issue
Block a user