diff --git a/BUILT.md b/BUILT.md index 0ecf9e3..c50a559 100644 --- a/BUILT.md +++ b/BUILT.md @@ -4159,15 +4159,11 @@ with `flan/watch-begin`, `flan/watch-end` and the four emit functions declared a already declares `flan_dev_emit*`. Nothing else has to move: `Render.render` is unchanged, the runtime side is built and tested, and the daemon and the editor cannot tell which kind of caller filled the table. -That arm is also what **ghost text** is gated on, which is the more interesting consequence. Values shown inline -beside the code they belong to need a *place*, and nothing in the table has one — `(watch-i64 "ticks" ticks)` says -what the value is called, not where it was written. A source location would have to be carried per entry, which means -the caller supplies it, which means the call site is generated rather than hand-written. A `declare-c` call cannot do -it: the program would have to pass its own `__FILE__` by hand and it would drift the moment the line moved. So the -composite renderer and ghost text want the same form, for different reasons. `flan-watch.el` records the other two -things ghost text would need — overlay invalidation as the buffer is edited, and a rule for a watch inside a loop, -which the buffer sidesteps by showing the last value written and which inline has no obvious answer that does not -become the query UI this design exists to avoid. +~~That arm is also what **ghost text** is gated on.~~ **It was not, and ghost text is built without it** — see +"Ghost text finds its anchor in the buffer, not in the table" below. The claim was that values shown inline need a +*place* and nothing in the table has one, so a source location would have to be carried per entry, so the call site +would have to be generated. True of the table and false of the conclusion: the call site is in the buffer. The +composite renderer still wants the form, for its own reason, and it is the only one of the two that does. ## An error is a value, and there is more than one of them @@ -4469,3 +4465,71 @@ between it and the target" — as a one-line gotcha. Those were rewritten in pla having a second bullet added beside them, and the same reasoning went into `spec-conditions.md` §5, which is the section that already enumerates what a transfer does and does not do: it runs `defer`s, it skips `errdefer`s, and it does not undo. No numbered case changed meaning, so the freeze holds. + +## Ghost text finds its anchor in the buffer, not in the table + +`M-x flan-watch-ghost-mode` paints each watched value inline, after the line holding the call that wrote it, as an +overlay `after-string`. It is an **addition** to the watch buffer and not a replacement, and both can be on at once: +the buffer is the picture you want when you want everything, inline is the picture you want when you are reading one +function. + +**Why it was thought to be blocked, and why it was not.** The earlier note in `flan-watch.el` — and two paragraphs +above, now struck — said ghost text needed a source location per table entry, which needs a generated call site, +which means the `(watch ...)` form in `check.ml`. Every step of that is true *of the table*: it holds a name and a +rendered string, and `(watch-i64 "ticks" ticks)` says what the value is called, not where it was written. The +conclusion did not follow. **The call site is in the buffer.** The name in the table is the string literal in that +call, so the editor searches its own text for the anchor instead of being told it. Nothing new is asked of the daemon, +and the composite-struct renderer is now the only one of the two things that really does want the checker arm. + +**The head of the call cannot be hardcoded.** `watch-i64` is a name the *program's* author chose in their own +`declare-c`; only `flan_dev_watch_i64` is fixed, and the editor never sees it. Hence +`flan-watch-ghost-call-regexp`. Matching on the string literal rather than on the head is what makes that safe — a +name in the table came from one of these call sites by construction — and the syntax check (`nth 8` of a +`syntax-ppss`) is what stops a call *written inside a string* being taken for one. + +**One reply, two pictures.** Ghost text does not poll. It is painted from `flan-watch--absorb`, the same function +that paints the buffer, from the same reply, so the two cannot disagree and there is no second `:op "watch"` in +flight — the one-request invariant `flan-dev-settle-hook` exists to keep. What had to change is that the **watch +buffer used to be the subscription**: killing it cancelled the timer and disarmed the table. That was right while it +was the only consumer and wrong the moment it was not, so arming and the timer now hang off +`flan-watch--consumers`, and only the last consumer out turns the lights off. + +**Overlays are replaced wholesale on every repaint**, never followed through edits. That is the entire answer to the +invalidation problem the earlier note called the work: a line that moved cannot strand an overlay, because no overlay +outlives a tick. The cost is bounded by scanning only buffers **shown in a window** — an overlay nobody can see is +worth nothing, and the project's other files are not touched. It also means a file you scroll to is annotated within +one tick with nothing to hook. + +**The questions the design had to settle, and the answers:** + +- **Two sites, one name.** Both get the overlay, both show the same value, and the text says `one slot, 2 sites`. The + table has one slot per name and the last writer in the frame wins, so that *is* the value at both. Showing it at one + site would imply the other was not running; showing it at both in silence would read as a coincidence. +- **A watch inside a loop** shows the last value written, exactly as the buffer does. "The last of 4000 iterations" is + often not the interesting one, and every better answer is a UI for building a query — the one thing this design + exists not to have. Settled, not open. +- **Stopped.** The values survive a break, which is half the reason for pushing rather than polling. But they are the + *last frame's*, and inline they sit in code that looks perfectly live with no modeline beside them, so each one says + `last frame` and changes face. The buffer needs no such marker: you opened it deliberately and `flan:stopped(...)` + is already in view. +- **A site with no row** is annotated only when the table reports **overflow**, where it says so. Then the value + exists and was dropped, which is the case silence would send you hunting for a bug over; the buffer's overflow line + says the same thing without being able to say *which* name. With room to spare, a site with no row has simply not + run yet, and annotating every one of those at startup is noise. +- **A name the runtime clipped.** It holds 31 bytes, so a longer name in the source never matches a row exactly. The + lookup falls back to a prefix match, guarded on the row being a full 31 bytes so a short name cannot claim a site + that merely starts with it. + +**What gets no ghost text, and it is the honest limit of anchoring on text:** a watch call produced by a macro, or one +whose name is not a literal. There is nothing in the source to find — the editor is reading the file, not debug info — +and both still appear in the watch buffer. + +**One bug worth recording**, because it cost a debugging session and reads as impossible: `syntax-ppss` moves point and +clobbers the match data. Calling it inside a `re-search-forward` loop and then reading `match-string` or +`line-end-position` puts the scan back where it started, and the loop never ends. Everything wanted from a match is +now read out before the check, and the loop advances to a saved `match-end`. + +`emacs/test-flan-watch.el` covers it, loaded from `test-flan-cider.el` for the reason `test-flan-mode.el` gives: +`emacs/*.el` is already a dependency of that dune stanza, so a new `.el` needs no build change. Nothing in it needs a +daemon — ghost text is a function from a table of rows and the text in a buffer to overlays, and both halves are +fixtures. diff --git a/NEXT.md b/NEXT.md index 4b56725..9398561 100644 --- a/NEXT.md +++ b/NEXT.md @@ -167,7 +167,7 @@ Both lanes committed their main work and died on trailing polish; both are merge instrumentation travels as a *separate field* applied by the daemon **after** parsing (splicing text would shift every source location after it), and the mark **sticks** until the form is evaluated plainly. The watch lane got as far as deciding to splice into the `Ast` rather than the forms. -- **Ghost text** for the watch window — values shown inline at the code they belong to — is noted and not designed. +- ~~**Ghost text** for the watch window — values shown inline at the code they belong to — is noted and not designed.~~ **Built.** See `BUILT.md`, "Ghost text finds its anchor in the buffer, not in the table". - `tools/unit-return.py` is re-runnable; run it over any `.flan` file a lane wrote before the conversion landed. # Where this is @@ -1150,14 +1150,16 @@ runtime entry points, which needs no compiler change at all. A struct or a slice type — one arm in `check.ml` beside `print`, which `BUILT.md` writes out in full — and that file is held by another lane, so it was left alone rather than reached into. -**Ghost text is gated on that same arm**, which is the finding worth keeping. It was raised here as an alternative or -addition to the buffer, and it turns out not to be independent: values shown inline need a *place*, and nothing in -the table has one — `(watch-i64 "ticks" ticks)` says what the value is called, not where it was written. Carrying a -source location means the caller supplies it, which means the call site is generated rather than hand-written. A -`declare-c` call cannot do it; the program would have to pass its own `__FILE__` by hand and it would drift the -moment the line moved. `flan-watch.el` records the other two things ghost text would need: invalidating overlays as -the buffer is edited, and a rule for a watch inside a loop — which the buffer sidesteps by showing the last value -written, and which inline has no obvious answer that does not turn into the query UI this design exists to avoid. +~~**Ghost text is gated on that same arm**, which is the finding worth keeping.~~ **Wrong, and ghost text is built.** +The reasoning was that values shown inline need a *place*, that nothing in the table has one — `(watch-i64 "ticks" +ticks)` says what the value is called, not where it was written — and that carrying a source location means the +caller supplies it, which means a generated call site. Every step of that is still true **of the table**, and the +conclusion did not follow: the call site is in the *buffer*, and the name in the table is the string literal in it, +so the anchor is searched for rather than reported. Nothing new is asked of the daemon. The two open questions +`flan-watch.el` recorded are answered rather than solved — overlays are replaced wholesale every repaint, so an edit +has nothing to invalidate, and a watch in a loop shows the last value written exactly as the buffer does, because +every better answer is the query UI this design exists to avoid. See `BUILT.md`, "Ghost text finds its anchor in the +buffer, not in the table". ~~**The inspector gets a second way to start: an address and a type.**~~ **Built.** See `BUILT.md`, "Two ways to root a walk, and why neither subsumes the other". It went in as a frame and a slot *index* rather than an address and a diff --git a/emacs/MANUAL.md b/emacs/MANUAL.md index e87929d..4718513 100644 --- a/emacs/MANUAL.md +++ b/emacs/MANUAL.md @@ -286,9 +286,64 @@ value was clipped. struct or a slice does not. That is not an oversight in the runtime — a Flan value carries no header, so rendering one is a walk over its *type* at compile time, and a `(watch "hp" hp)` form in the compiler is what would do that walk. -It is not built. Ghost text — values shown inline beside the code they came -from — is a further question and needs the same form, because nothing in the -table carries a source location. `flan-watch.el` says what both would need. +It is not built. `flan-watch.el` says what it would need. + +### Ghost text — the same values, inline + +**`M-x flan-watch-ghost-mode`** shows each watched value *at the call that wrote +it*, as faint text after the line, instead of in a buffer of its own: + +```flan +(defn step [] i64 + (set ticks (+ ticks 1)) + (watch-i64 "ticks" ticks) => 4812 + ticks) +``` + +It is an **addition, not a replacement**, and the two can be on together. The +buffer is what you want when you want everything at once; inline is what you +want when you are reading one function and the value belongs beside the code +rather than three windows away. `M-x flan-watch-ghost-mode` again turns it off, +and turning either one off leaves the other running. + +**How it finds the place.** The table carries a name and a rendered string and +no source location — that would need the `(watch ...)` form above. It does not +have to: `(watch-i64 "ticks" ticks)` is *in the buffer you are looking at*, and +the name in the table is the string literal in it, so the call site is found by +searching the text. Nothing new is asked of the running program. If you bound +your own entry points to names that do not start with `watch`, set +`flan-watch-ghost-call-regexp` — the Flan name is yours, and only the C symbol +behind it is fixed. + +**When it updates.** On the same timer as the buffer, from the same reply, so +the two cannot disagree. As with the buffer, the interval decides how often the +picture is repainted and not how fresh it is. Overlays are replaced wholesale +each repaint rather than followed through your edits, so a line you moved never +leaves one stranded, and a file you scroll to gets its values within a tick. +Only buffers **shown in a window** are scanned, which is what keeps that cheap. + +**One name, two call sites.** Both show it, and both say `one slot, 2 sites`. +That is the truth rather than a failure to choose: the table has one slot per +name and the last call in the frame wins, so the value really is the same at +both, and it is whichever ran last. In a **loop**, you see the last value +written — the same answer the buffer gives. "The last of 4000 iterations" is +often not the one you wanted, but every better answer is a UI for building a +query, which is the thing this design exists not to have. + +**While the program is stopped**, the values are the last frame's — that is the +point of pushing rather than polling — and each one says `last frame` and changes +colour. The watch buffer does not need to, because you opened it on purpose and +`flan:stopped(...)` is already in the modeline; inline, the value sits in the +middle of code that looks perfectly live. + +**What gets no ghost text.** A watch call produced by a macro, or one whose name +is not a literal: there is nothing in the source to find, and the editor is +reading your file rather than debug info. Those still appear in the watch +buffer. A site you have typed but not yet installed with `C-c C-c` has no value +yet either — that is the ordinary stale-caller situation the modeline already +tracks. And when the table is **full**, a site that found no slot says so rather +than showing nothing, which is the one case where silence would send you looking +for a bug in your program. --- @@ -452,7 +507,8 @@ Use `C-c C-g` if you need frames. | `M-.` / `M-,` | where a name is written / back | Commands with no key: `M-x flan-dev` (start a program), `M-x flan-dev-quit` -(stop it), `M-x flan-watch` (the watch buffer) and `M-x flan-watch-stop`. +(stop it), `M-x flan-watch` (the watch buffer), `M-x flan-watch-stop`, and +`M-x flan-watch-ghost-mode` (the same values inline). --- @@ -470,6 +526,7 @@ Commands with no key: `M-x flan-dev` (start a program), `M-x flan-dev-quit` | `flan-dev-start-timeout` | `60` | seconds to wait for a program to come up | | `flan-watch-buffer` | `"*flan-watch*"` | where watched values are painted | | `flan-watch-interval` | `0.2` | seconds between repaints — not the watch rate | +| `flan-watch-ghost-call-regexp` | `"watch\(?:-[[:alnum:]]+\)?"` | the head of a call ghost text anchors on | --- @@ -480,7 +537,7 @@ Commands with no key: `M-x flan-dev` (start a program), `M-x flan-dev-quit` | `flan-mode.el` | the major mode: syntax, indentation, imenu, the keymap | | `flan-dev.el` | the client — the socket, evaluation, xref, eldoc, completion | | `flan-repl.el` | the `*flan-repl*` buffer | -| `flan-watch.el` | the watch buffer: the program pushes, this paints | +| `flan-watch.el` | watched values: the program pushes, this paints them in a buffer and inline | | `flan-cnr.el` | the conditions-and-restarts buffer | | `flan-inspect.el` | the value inspector | | `flan-dape.el` | lldb through dape; optional | diff --git a/emacs/flan-watch.el b/emacs/flan-watch.el index 73b8180..bcca93a 100644 --- a/emacs/flan-watch.el +++ b/emacs/flan-watch.el @@ -1,9 +1,14 @@ -;;; flan-watch.el --- A pinned, self-overwriting watch buffer -*- lexical-binding: t; -*- +;;; flan-watch.el --- Watched values, in a pinned buffer or inline -*- lexical-binding: t; -*- ;; A HUD for a running Flan program: a buffer that always shows the current ;; frame's values and nothing else. Ported from the author's Clojure ;; `clj-watch', with one thing kept and one thing inverted. ;; +;; There are two pictures of one table. The buffer is below; ghost text — +;; `flan-watch-ghost-mode', the same values shown inline at the calls that wrote +;; them — is further down, with its own rationale. Everything between here and +;; there is common to both. +;; ;; KEPT, and it is the good idea in the original: **the program decides what is ;; shown**. There is no watch-expression machinery here, no per-variable ;; registration, no UI for building a query. The program says what it wants @@ -79,6 +84,14 @@ refreshed, which is why a slow value here costs freshness and nothing else." "Non-nil while a watch request is out and its reply has not been read.") (defvar flan-watch--rows nil "The last table painted, as a list of (NAME . VALUE).") +(defvar flan-watch--consumers nil + "Which pictures of the table are currently wanted: `buffer', `ghost', or both. + +There is one table, one arming message and one timer, and more than one way to +look at what they produce. Holding the subscription here rather than in the +watch buffer is what lets ghost text outlive that buffer being closed — the +original design made the buffer *be* the subscription, which was right when it +was the only consumer and wrong the moment it was not.") ;;; Painting @@ -131,6 +144,181 @@ refreshed, which is why a slow value here costs freshness and nothing else." ;; in it — look at a particular line while the program runs. (replace-buffer-contents tmp)))))) +;;; Ghost text — the same table, painted at the code instead of beside it + +;; The buffer shows everything at once, which is what you want while the game +;; runs. Ghost text shows one value where it was written, which is what you +;; want while you read one function. They are two pictures of one table, so +;; this is an addition and not a replacement, and neither one polls separately: +;; both are painted from the same reply, in `flan-watch--absorb'. +;; +;; WHERE A VALUE ATTACHES. The table carries a name and a rendered string and +;; no source location, and it is not going to grow one here — that needs a +;; `(watch ...)' form in the checker, which is a file this does not own. But a +;; location is not actually missing: `(watch-i64 "hp" hp)' is *in the buffer*, +;; and the name in the table is the string literal in it. So the anchor is +;; found by searching the text rather than by being told, which costs one +;; regexp scan per displayed buffer per repaint and needs nothing new from the +;; daemon. +;; +;; The head of that call cannot be hardcoded. `watch-i64' is a name the +;; program's author chose in their own `declare-c'; only the C symbol behind it +;; is fixed, and the editor never sees that. Hence +;; `flan-watch-ghost-call-regexp', whose default matches the shape the manual +;; suggests and which anyone who named theirs differently can change. Matching +;; the string literal and not the head is what makes that safe: a name in the +;; table came from one of these call sites by construction. +;; +;; WHEN IT UPDATES. On the same timer, from the same reply. The two pictures +;; therefore cannot disagree — they are one table read, painted twice — and +;; there is no second `:op "watch"' in flight, which is the invariant +;; `flan-dev-settle-hook' exists to keep. As with the buffer, the timer decides +;; how often the picture is repainted and not how fresh it is: the program +;; writes every frame regardless. +;; +;; Overlays are deleted and re-placed from scratch on every repaint rather than +;; being tracked across edits. That is the whole answer to the invalidation +;; problem the earlier sketch of this called the work: a line that moved does +;; not leave an overlay behind, because no overlay outlives a tick. The cost is +;; bounded by only scanning buffers that are *shown in a window* — an overlay in +;; a buffer nobody is looking at is invisible anyway, and a project's other +;; hundred files are not scanned. It also means a file you scroll to gets its +;; ghost text within one tick with nothing to hook. +;; +;; TWO SITES, ONE NAME. Both get the overlay and both show the same value, and +;; the text says so. That is the honest picture rather than a failure to pick: +;; the table has one slot per name and the last writer in the frame wins, so +;; the value at both sites really is the same value, and it is whichever call +;; ran last. Silently showing it at one site would suggest the other is not +;; running; showing it at both without a word would read as a coincidence. +;; +;; A LOOP. Inline shows the last value written, exactly as the buffer does. +;; "The last of 4000 iterations" is usually not the interesting one, but every +;; better answer is a UI for building a query, which is the thing this design +;; exists not to have. Settled, not open. +;; +;; STOPPED. The values survive a break — that is a property of pushing rather +;; than polling, and half the reason for it. But they are the *last frame's*, +;; and inline they sit in the middle of live-looking code with no modeline next +;; to them, so they say "last frame" in words and change face. The watch buffer +;; does not need to: you opened it deliberately and `flan:stopped(...)' is +;; already in view. + +(defcustom flan-watch-ghost-call-regexp "watch\\(?:-[[:alnum:]]+\\)?" + "Regexp matching the head of a call that writes the watch table. + +Matched against the symbol after an open paren, with the watched name as a +string literal after it. The default covers `watch', `watch-i64', `watch-f64' +and `watch-str'. It is a setting because the Flan name is the program author's +own `declare-c' binding — only the C symbol behind it is fixed, and the editor +never sees that." + :type 'regexp) + +(defface flan-watch-ghost-face '((t :inherit shadow)) + "Face for an inline watched value while the program is running.") + +(defface flan-watch-ghost-stale-face '((t :inherit warning)) + "Face for an inline watched value while the program is stopped. + +Distinct from the running face because the value is the last frame's, and +inline it has no modeline beside it to say so.") + +(defvar flan-watch--ghost-overlays nil + "Every overlay this has placed, across every buffer.") + +(defun flan-watch--ghost-clear () + "Remove every overlay this has placed." + (mapc #'delete-overlay flan-watch--ghost-overlays) + (setq flan-watch--ghost-overlays nil)) + +(defun flan-watch--ghost-buffers () + "Flan buffers currently shown in some window, on any frame." + (let (bufs) + (dolist (w (window-list-1 nil 'nomini t)) + (let ((b (window-buffer w))) + (when (and (eq (buffer-local-value 'major-mode b) 'flan-mode) + (not (memq b bufs))) + (push b bufs)))) + bufs)) + +(defun flan-watch--ghost-sites () + "Watch call sites in the current buffer, as a list of (NAME . END-OF-LINE)." + (let ((re (concat "(\\s-*" flan-watch-ghost-call-regexp + "\\s-+\"\\([^\"\n]*\\)\"")) + (sites nil)) + (save-excursion + (goto-char (point-min)) + (while (re-search-forward re nil t) + ;; Everything wanted from this match is read out *before* the check. + ;; `syntax-ppss' moves point and clobbers the match data, so reading + ;; either afterwards puts the scan back where it started and the loop + ;; never ends. Found the hard way. + (let ((beg (match-beginning 0)) + (end (match-end 0)) + (name (match-string-no-properties 1)) + (eol (line-end-position))) + ;; A call inside a string or a comment is text that looks like code. + (unless (nth 8 (syntax-ppss beg)) + (push (cons name eol) sites)) + (goto-char end)))) + (nreverse sites))) + +(defun flan-watch--ghost-row (name rows) + "The row in ROWS for the site name NAME, or nil. + +Falls back to a prefix match against a name the runtime clipped: it holds 31 +bytes of a name, so a longer one in the source will never match it exactly. +Guarded on the row's length so a short name cannot be claimed by accident." + (or (assoc name rows) + (let (hit) + (dolist (r rows hit) + (when (and (not hit) + (>= (string-bytes (car r)) 31) + (string-prefix-p (car r) name)) + (setq hit r)))))) + +(defun flan-watch--ghost-text (value sites stale) + "The after-string for a value: VALUE, seen at SITES places, STALE if stopped." + (propertize + (concat " => " value + (when (> sites 1) (format " one slot, %d sites" sites)) + (when stale " last frame")) + 'face (if stale 'flan-watch-ghost-stale-face 'flan-watch-ghost-face) + ;; Without this, point at end of line lands on the ghost text rather than + ;; on the buffer's own last column. + 'cursor t)) + +(defun flan-watch--ghost-paint (rows overflow) + "Place inline overlays for ROWS. OVERFLOW means some name found no slot." + (flan-watch--ghost-clear) + (let ((stale (and flan-dev--stopped t))) + (dolist (buf (flan-watch--ghost-buffers)) + (with-current-buffer buf + (let* ((sites (flan-watch--ghost-sites)) + (counts (make-hash-table :test #'equal))) + (dolist (s sites) + (puthash (car s) (1+ (gethash (car s) counts 0)) counts)) + (dolist (s sites) + (let* ((row (flan-watch--ghost-row (car s) rows)) + (text + (cond + (row (flan-watch--ghost-text + (cdr row) (gethash (car s) counts 1) stale)) + ;; A site with no row and a full table is a value that + ;; exists and was dropped, which is the one case worth + ;; saying out loud — the buffer's overflow line says the + ;; same thing without being able to say *which*. A site + ;; with no row and room to spare has simply not run yet, + ;; and annotating every one of those at startup would be + ;; noise. + (overflow (flan-watch--ghost-text + "no slot: the table is full" 1 stale))))) + (when text + (let ((ov (make-overlay (cdr s) (cdr s)))) + (overlay-put ov 'after-string text) + (overlay-put ov 'flan-watch-ghost t) + (push ov flan-watch--ghost-overlays)))))))))) + ;;; The tick (defun flan-watch--absorb (reply) @@ -141,9 +329,17 @@ refreshed, which is why a slow value here costs freshness and nothing else." (mapcar (lambda (r) (cons (nth 0 r) (nth 1 r))) (plist-get reply :watch))) (flan-watch--paint - (flan-watch--format flan-watch--rows (plist-get reply :overflow)))) - (_ (flan-watch--paint - (format "error:\n%s\n" (or (plist-get reply :message) "refused")))))) + (flan-watch--format flan-watch--rows (plist-get reply :overflow))) + (when (memq 'ghost flan-watch--consumers) + (flan-watch--ghost-paint flan-watch--rows (plist-get reply :overflow)))) + (_ + ;; The overlays go rather than being left at their last values. Inline + ;; there is nowhere to print the error, so a stale number beside live code + ;; would be the whole picture and it would be wrong; the watch buffer, if + ;; it is open, says what happened. + (flan-watch--ghost-clear) + (flan-watch--paint + (format "error:\n%s\n" (or (plist-get reply :message) "refused")))))) (defun flan-watch--settle () "Collect an outstanding watch reply, blocking if it has not arrived. @@ -167,10 +363,14 @@ the one caller that must not. So this takes whatever has already arrived and sends the next question, leaving at most one request in flight — the invariant `flan-dev-settle-hook' exists to keep." (cond - ;; The buffer is the subscription. Killing it stops the timer and disarms - ;; the table, so a program whose watch buffer is closed is back to paying a - ;; load and a branch per watch call. - ((not (get-buffer flan-watch-buffer)) (flan-watch-stop)) + ;; Killing the buffer cancels the buffer's half of the subscription, and + ;; only that. When it was the only consumer this stops the timer and + ;; disarms the table, so a program nobody is watching is back to paying a + ;; load and a branch per watch call; when ghost text is also on, the table + ;; stays armed and the ticks carry on feeding it. + ((and (memq 'buffer flan-watch--consumers) + (not (get-buffer flan-watch-buffer))) + (flan-watch--drop 'buffer)) ((not (process-live-p flan-dev--connection)) (flan-watch--paint "error:\nnot connected to a running program\n") (flan-watch-stop)) @@ -195,18 +395,37 @@ sends the next question, leaving at most one request in flight — the invariant "Major mode for the pinned watch buffer." (setq-local truncate-lines t)) +(defun flan-watch--subscribe (consumer) + "Arm the table for CONSUMER and make sure the shared timer is running. + +Arming is a message, not something the daemon infers. The program is the +writer, so it has to be told somebody is looking — and while nobody is, nothing +writes the table at all, which is what makes a watch call in a program nobody is +debugging a load and a not-taken branch. It is sent once for the first +consumer: two of them looking at one table is still one table." + (flan-dev--live-connection) + (unless flan-watch--consumers + (let ((r (flan-dev--request '(:op "watch-enable" :on t)))) + (unless (equal (plist-get r :status) "ok") + (user-error "flan: %s" (or (plist-get r :message) "watch refused"))))) + (unless (memq consumer flan-watch--consumers) + (push consumer flan-watch--consumers)) + (add-hook 'flan-dev-settle-hook #'flan-watch--settle) + (when flan-watch--timer (cancel-timer flan-watch--timer)) + (setq flan-watch--timer + (run-with-timer 0 flan-watch-interval #'flan-watch--tick))) + +(defun flan-watch--drop (consumer) + "Stop painting CONSUMER, and tear everything down if it was the last one." + (setq flan-watch--consumers (delq consumer flan-watch--consumers)) + (when (eq consumer 'ghost) (flan-watch--ghost-clear)) + (unless flan-watch--consumers (flan-watch-stop))) + ;;;###autoload (defun flan-watch () "Open the watch buffer and start painting the running program's values." (interactive) - (flan-dev--live-connection) - ;; Arming is a message, not something the daemon infers. The program is the - ;; writer, so it has to be told somebody is looking — and while nobody is, - ;; nothing writes the table at all, which is what makes a watch call in a - ;; program nobody is debugging a load and a not-taken branch. - (let ((r (flan-dev--request '(:op "watch-enable" :on t)))) - (unless (equal (plist-get r :status) "ok") - (user-error "flan: %s" (or (plist-get r :message) "watch refused")))) + (flan-watch--subscribe 'buffer) (with-current-buffer (get-buffer-create flan-watch-buffer) (unless (eq major-mode 'flan-watch-mode) (flan-watch-mode))) ;; Painted before the first reply, rather than left blank until one arrives. @@ -214,15 +433,35 @@ sends the next question, leaving at most one request in flight — the invariant ;; reason for it here is the honest one — the program is not calling into the ;; table — which is worth saying in words rather than by showing nothing. (flan-watch--paint (flan-watch--format nil 0)) - (add-hook 'flan-dev-settle-hook #'flan-watch--settle) - (when flan-watch--timer (cancel-timer flan-watch--timer)) - (setq flan-watch--timer - (run-with-timer 0 flan-watch-interval #'flan-watch--tick)) (display-buffer flan-watch-buffer)) +;;;###autoload +(define-minor-mode flan-watch-ghost-mode + "Show watched values inline, beside the calls that wrote them. + +An addition to `flan-watch' rather than a replacement: the buffer is what you +want when you want everything at once, and this is what you want when you are +reading one function. Both are painted from the same table read, so they +cannot disagree, and turning either off leaves the other running." + :global t + :lighter " flan-ghost" + (if flan-watch-ghost-mode + ;; A refused arming leaves the mode off rather than on-and-doing-nothing. + (condition-case err + (flan-watch--subscribe 'ghost) + (error (setq flan-watch-ghost-mode nil) (signal (car err) (cdr err)))) + (flan-watch--drop 'ghost))) + (defun flan-watch-stop () - "Stop painting, and tell the program to stop writing the table." + "Stop painting, and tell the program to stop writing the table. + +Tears down both consumers. `flan-watch--drop' is the way to stop one of them." (interactive) + (setq flan-watch--consumers nil) + ;; Directly, not through the mode function: that would call back into + ;; `flan-watch--drop' and back into here. + (setq flan-watch-ghost-mode nil) + (flan-watch--ghost-clear) (when flan-watch--timer (cancel-timer flan-watch--timer) (setq flan-watch--timer nil)) @@ -239,38 +478,36 @@ sends the next question, leaving at most one request in flight — the invariant (when (process-live-p flan-dev--connection) (ignore-errors (flan-dev--request '(:op "watch-enable" :on nil))))) -;;; Ghost text — not built, and what it would need +;;; What ghost text still cannot show -;; The author raised showing values *inline at the code they belong to* rather -;; than in a buffer of their own. It is a better picture and it is a different -;; feature, so it is written down here rather than half-done. +;; Built, above, on a buffer search rather than on a location from the runtime +;; — see the rationale there. An earlier note in this file said ghost text was +;; gated on a `(watch ...)' form in the checker, because nothing in the table +;; carries a file:line:col. That is still true of the *table* and it turned out +;; not to be the blocker: the call site is in the buffer, and the name in the +;; table is the string literal in it, so the anchor is searched for instead of +;; being told. Nothing new is asked of the daemon. ;; -;; What the buffer needs is a name and a string. Ghost text needs a *place*, -;; and nothing in the table has one: `(watch-i64 "ticks" ticks)' says what the -;; value is called, not where it was written. Three things would have to be -;; added, and the first is the real one: +;; What the search cannot do, and what it costs: ;; -;; 1. A source location per entry. The runtime would have to carry a -;; file:line:col alongside the name, which means the *caller* supplies it, -;; which means the call site is generated rather than hand-written — i.e. -;; the `(watch ...)' form in the checker, which is where a form's own -;; location is already known. A declare-c call cannot do it: the program -;; would have to pass __FILE__ by hand and it would drift the moment the -;; line moved. So ghost text is gated on the same check.ml arm the -;; composite renderer is. +;; - A watch call written by a macro has no literal to find, so it gets no +;; ghost text. The watch buffer still shows it. This is the honest limit +;; of anchoring on text: the editor is reading source, not debug info, and +;; a form that only exists after expansion is not in the source. ;; -;; 2. Overlays keyed to that location, with `after-string', refreshed on the -;; same timer. Cheap once (1) exists; the work is invalidating them when -;; the buffer is edited, since a line that moved leaves its overlay behind. +;; - A name assembled at run time — anything that is not a literal at the call +;; site — is the same case for the same reason. ;; -;; 3. A rule for a watch inside a loop, which the buffer sidesteps by showing -;; the last value written. Inline, "the last of 4000 iterations" is -;; usually not the interesting one, and there is no obvious better answer -;; that does not become a UI for building a query — which is the thing -;; this design exists to avoid. +;; - The module in the running program can be older than the buffer. A site +;; you have typed but not yet installed with `C-c C-c' has no row, and one +;; you deleted but not yet installed still writes. Deliberately unreported: +;; the stale-caller machinery in `flan-dev.el' already tracks what needs +;; re-evaluating, and a second opinion about it here would be a worse one. ;; -;; (3) is why this is a question and not a task. (1) is why it cannot be -;; started here. +;; Still genuinely blocked on the checker, and unchanged by any of this: a +;; **struct or a slice**. Rendering one is a compile-time walk over its type, +;; the runtime has scalar entry points only, and no amount of searching the +;; buffer produces a value the table does not hold. See BUILT.md. (provide 'flan-watch) ;;; flan-watch.el ends here diff --git a/emacs/test-flan-cider.el b/emacs/test-flan-cider.el index 2054e80..ca19c40 100644 --- a/emacs/test-flan-cider.el +++ b/emacs/test-flan-cider.el @@ -1008,6 +1008,13 @@ stopped program, which is the case where it should fire." (file-name-directory load-file-name)) nil t) +;; Ghost text, which is the same kind of thing: rows in, overlays out, and the +;; buffer it reads is a fixture like any other reply here. Loaded for the same +;; reason. +(load (expand-file-name "test-flan-watch.el" + (file-name-directory load-file-name)) + nil t) + (message "\n%d checks, %d failures" test-flan--ran test-flan--failures) (kill-emacs (if (> test-flan--failures 0) 1 0)) diff --git a/emacs/test-flan-watch.el b/emacs/test-flan-watch.el new file mode 100644 index 0000000..6e7a21c --- /dev/null +++ b/emacs/test-flan-watch.el @@ -0,0 +1,183 @@ +;;; test-flan-watch.el --- Ghost text, from a buffer and a fixture table -*- lexical-binding: t; -*- + +;; Loaded by test-flan-cider.el, which runs under `dune test', for the reason +;; test-flan-mode.el gives: `emacs/*.el' is already a dependency of that +;; stanza, so a file here needs no build change to be run. +;; +;; Nothing here needs a daemon. Ghost text is a function from *a table of rows* +;; and *the text in a buffer* to *overlays*, and both halves are fixtures: the +;; rows are what a watch reply carries, and the buffer is Flan source. The +;; interesting failures are all on that side — a call site inside a string taken +;; for code, a name the runtime clipped no longer matching the site that wrote +;; it, an overlay left behind from the last repaint. +;; +;; The one case that is not about text is the subscription: the watch buffer +;; used to *be* the subscription, and ghost text is the second consumer that +;; made that wrong. Closing one must leave the other running. + +;;; Code: + +(require 'flan-mode) +(require 'flan-watch) + +(declare-function test-flan--check "test-flan-cider" (name ok)) + +(defun test-flan-watch--buffer (text) + "A live `flan-mode' buffer holding TEXT, shown in a window. + +Shown because `flan-watch--ghost-paint' only scans displayed buffers, which is +what bounds its cost; a `with-temp-buffer' would be scanned by nothing." + (let ((buf (get-buffer-create "test-flan-watch.flan"))) + (with-current-buffer buf + (let ((inhibit-read-only t)) + (erase-buffer) + (insert text)) + (flan-mode)) + (set-window-buffer (selected-window) buf) + buf)) + +(defun test-flan-watch--starts () + "Where the overlays sit, in buffer order." + (sort (mapcar #'overlay-start flan-watch--ghost-overlays) #'<)) + +(defun test-flan-watch--strings () + "What the overlays say, in buffer order." + (mapcar (lambda (ov) (substring-no-properties (overlay-get ov 'after-string))) + (sort (copy-sequence flan-watch--ghost-overlays) + (lambda (a b) (< (overlay-start a) (overlay-start b)))))) + +;; --- Finding the call site ------------------------------------------------ +;; +;; The anchor is searched for rather than reported, because the table carries a +;; name and no location. A call inside a string is the case that makes the +;; syntax check earn its place: it is text that looks exactly like code. + +(let ((buf (test-flan-watch--buffer + (concat "(defn step [] i64\n" + " (watch-i64 \"ticks\" ticks)\n" + " (watch-f64 \"hp\" hp)\n" + " (println \"(watch-i64 \\\"fake\\\" x)\")\n" + " (watch \"hp\" hp))\n")))) + (with-current-buffer buf + (let ((sites (flan-watch--ghost-sites))) + (test-flan--check + "every watch call in the buffer is found, by the name it writes" + (equal (mapcar #'car sites) '("ticks" "hp" "hp"))) + (test-flan--check + "and a call written inside a string is not one of them" + (not (member "fake" (mapcar #'car sites))))))) + +;; --- Matching a row to a site --------------------------------------------- +;; +;; The runtime holds 31 bytes of a name, so a longer one in the source can +;; never match a row exactly. The prefix fallback is guarded on the row being +;; a full 31 bytes, or a short name would claim any site that starts with it. + +(let ((long (make-string 40 ?a))) + (test-flan--check + "a name the runtime clipped at 31 bytes still matches the site that wrote it" + (equal (flan-watch--ghost-row long (list (cons (make-string 31 ?a) "9"))) + (cons (make-string 31 ?a) "9"))) + (test-flan--check + "and a row short enough not to have been clipped claims only its own site" + (null (flan-watch--ghost-row "hpx" '(("hp" . "1")))))) + +;; --- What an overlay says ------------------------------------------------- + +(test-flan--check "a running value is just the value" + (equal (substring-no-properties + (flan-watch--ghost-text "42" 1 nil)) + " => 42")) +(test-flan--check + "two sites of one name each say so, because the table has one slot" + (string-match-p "one slot, 2 sites" + (flan-watch--ghost-text "42" 2 nil))) +(test-flan--check + "and one site says nothing about sites" + (not (string-match-p "sites" (flan-watch--ghost-text "42" 1 nil)))) +(test-flan--check + "a stopped program's value says it is the last frame's" + (string-match-p "last frame" (flan-watch--ghost-text "42" 1 t))) +(test-flan--check + "and is drawn in the face that is not the running one" + (eq (get-text-property 0 'face (flan-watch--ghost-text "42" 1 t)) + 'flan-watch-ghost-stale-face)) + +;; --- Painting ------------------------------------------------------------- + +(let ((buf (test-flan-watch--buffer + (concat "(watch-i64 \"ticks\" ticks)\n" + "(watch-i64 \"ticks\" ticks)\n" + "(watch-i64 \"gone\" g)\n"))) + (flan-watch--consumers '(ghost)) + (flan-dev--stopped nil)) + (flan-watch--ghost-paint '(("ticks" . "42")) nil) + (test-flan--check + "a site whose name is in the table gets an overlay, and one that is not does not" + (equal (test-flan-watch--strings) + '(" => 42 one slot, 2 sites" " => 42 one slot, 2 sites"))) + (test-flan--check + "each one sits at the end of its own call's line" + (equal (test-flan-watch--starts) + (with-current-buffer buf + (save-excursion (goto-char (point-min)) + (list (line-end-position 1) (line-end-position 2)))))) + + ;; The one case worth annotating a site with no row: the value exists and was + ;; dropped. The watch buffer's overflow line says the same thing without + ;; being able to say which name it happened to. + (flan-watch--ghost-paint '(("ticks" . "42")) t) + (test-flan--check + "with the table full, the site that found no slot is told so rather than left blank" + (equal (nth 2 (test-flan-watch--strings)) + " => no slot: the table is full")) + + (setq flan-dev--stopped "BoundsError") + (flan-watch--ghost-paint '(("ticks" . "42")) nil) + (test-flan--check + "stopping the program marks every overlay, not the buffer's modeline only" + (equal (test-flan-watch--strings) + '(" => 42 one slot, 2 sites last frame" + " => 42 one slot, 2 sites last frame"))) + (setq flan-dev--stopped nil) + + ;; The whole answer to invalidating an overlay whose line moved: no overlay + ;; outlives a repaint, so there is nothing to invalidate. + (with-current-buffer buf + (let ((inhibit-read-only t)) + (save-excursion (goto-char (point-min)) (insert "\n\n")))) + (flan-watch--ghost-paint '(("ticks" . "42")) nil) + (test-flan--check + "editing the buffer strands nothing: a repaint replaces every overlay" + (equal (test-flan-watch--starts) + (with-current-buffer buf + (save-excursion (goto-char (point-min)) + (list (line-end-position 3) (line-end-position 4)))))) + + ;; A refusal clears them. Inline there is nowhere to print an error, so a + ;; stale number beside live code would be the whole picture and it would lie. + (flan-watch--absorb '(:status "error" :message "no")) + (test-flan--check "a refused reply takes the overlays with it" + (null flan-watch--ghost-overlays)) + (test-flan--check "and leaves none behind in the buffer" + (with-current-buffer buf + (null (overlays-in (point-min) (point-max)))))) + +;; --- The subscription ----------------------------------------------------- +;; +;; The buffer used to be the subscription, so killing it disarmed the table. +;; With two consumers that is wrong: closing one must leave the other armed, +;; and only the last one out turns the lights off. + +(let ((flan-watch--consumers '(buffer ghost)) + (torn nil)) + (cl-letf (((symbol-function 'flan-watch-stop) (lambda () (setq torn t)))) + (flan-watch--drop 'buffer) + (test-flan--check "closing the watch buffer leaves ghost text armed" + (and (equal flan-watch--consumers '(ghost)) (not torn))) + (flan-watch--drop 'ghost) + (test-flan--check "and the last consumer out disarms the table" + (and (null flan-watch--consumers) torn)))) + +(provide 'test-flan-watch) +;;; test-flan-watch.el ends here