Clear the pending queue on reset and drain it under unwind-protect
tp-reactive-reset cleared the four reactive registries and the buffer registry but not the global tp--batch-update-pending queue. Entries queued there by a nested variable write during a non-batched update were flushed by a tail that sat OUTSIDE any unwind-protect, so an error escaping the re-render (for example from a modification hook) stranded them; a later tp-layer-reset left the ghost entries naming layers that no longer existed, and the next completed update's flush replayed them against the fresh registry. Fix: - tp-reactive-reset now also clears tp--batch-update-pending (and so does tp-layer-reset, which delegates to it). - tp--reactive-apply-update wraps the non-nested update body in unwind-protect with the existing flush tail as the cleanup form, so nested queue entries flush even when the re-render signals; the reentrancy guard is unbound by then, so the cleanup flush re-renders exactly like the success path. Tests port the review's probes: a queued ghost entry disappears on tp-reactive-reset, and a before-change-functions hook that writes a second reactive variable and then signals leaves the queue drained with the nested variable's re-render applied. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
This commit is contained in:
parent
a0d0e71ff8
commit
8f50231695
@ -483,6 +483,10 @@ it to allow re-definition to change initial values."
|
||||
(setq tp-layer-watchers nil)
|
||||
(setq tp-layer-computed nil)
|
||||
(setq tp-layer-data nil)
|
||||
;; Drop queued re-renders too: entries stranded by an error escaping
|
||||
;; an update would otherwise survive the reset and replay against
|
||||
;; freshly (re)defined layers on the next flush (ARCH-4).
|
||||
(setq tp--batch-update-pending nil)
|
||||
(clrhash tp--layer-buffers))
|
||||
|
||||
(provide 'tp-reactive)
|
||||
|
||||
@ -661,6 +661,64 @@ inside the replacement span."
|
||||
"emacs"))
|
||||
(should (eq (get-text-property 1 'face) 'bold)))))
|
||||
|
||||
;;; ARCH-4: the pending queue must survive neither reset nor errors
|
||||
|
||||
(defvar tp-rt-a4-face nil)
|
||||
(defvar tp-rt-a4-color nil)
|
||||
|
||||
(ert-deftest tp-render-test-reactive-reset-clears-pending-queue ()
|
||||
"tp-reactive-reset drops queued batch re-renders (ARCH-4).
|
||||
Stranded entries would otherwise survive the reset and replay against
|
||||
freshly (re)defined layers on the next flush."
|
||||
(unwind-protect
|
||||
(progn
|
||||
(tp--queue-batch-update 'tp-rt-a4-ghost 'tp-rt-a4-ghost-var nil nil)
|
||||
(should tp--batch-update-pending)
|
||||
(tp-reactive-reset)
|
||||
(should (null tp--batch-update-pending)))
|
||||
(setq tp--batch-update-pending nil)))
|
||||
|
||||
(ert-deftest tp-render-test-error-escaping-update-flushes-nested-queue ()
|
||||
"An error escaping a re-render cannot strand nested queued updates.
|
||||
A modification hook that writes a second reactive variable and then
|
||||
signals used to strand the nested entry in the global queue - the
|
||||
flush tail sat outside any unwind-protect. The flush now runs as the
|
||||
update unwinds, so the nested variable's re-render still lands and
|
||||
the queue is drained (ARCH-4)."
|
||||
(setq tp-rt-a4-face 'bold
|
||||
tp-rt-a4-color "red")
|
||||
(unwind-protect
|
||||
(progn
|
||||
(define-tp tp-rt-a4-layer-a () '(face $tp-rt-a4-face))
|
||||
(define-tp tp-rt-a4-layer-b ()
|
||||
'(face (:foreground $tp-rt-a4-color)))
|
||||
(with-temp-buffer
|
||||
(insert "Hello world")
|
||||
(tp-set 1 6 'tp-rt-a4-layer-a)
|
||||
(tp-set 7 12 'tp-rt-a4-layer-b)
|
||||
(let ((armed t))
|
||||
(add-hook 'before-change-functions
|
||||
(lambda (_beg _end)
|
||||
(when armed
|
||||
(setq armed nil)
|
||||
;; Nested reactive write from within the
|
||||
;; re-render: goes to the global queue.
|
||||
(setq tp-rt-a4-color "green")
|
||||
(error "boom from modification hook")))
|
||||
nil t)
|
||||
(should-error (setq tp-rt-a4-face 'italic))
|
||||
;; The nested entry was flushed on the way out, not
|
||||
;; stranded...
|
||||
(should (null tp--batch-update-pending))
|
||||
;; ...and its re-render landed despite the error.
|
||||
(should (equal (get-text-property 7 'face)
|
||||
'(:foreground "green"))))))
|
||||
(tp-undefine-layer 'tp-rt-a4-layer-a)
|
||||
(tp-undefine-layer 'tp-rt-a4-layer-b)
|
||||
(setq tp-rt-a4-face nil
|
||||
tp-rt-a4-color nil
|
||||
tp--batch-update-pending nil)))
|
||||
|
||||
;;; R3 (0.3.0): anonymous-layer garbage collection
|
||||
|
||||
(ert-deftest tp-render-test-gc-collects-unreferenced-anonymous-layer ()
|
||||
|
||||
80
tp-render.el
80
tp-render.el
@ -668,43 +668,49 @@ installed as `tp--reactive-update-function'."
|
||||
(if tp--reactive-updating
|
||||
;; Nested change fired from within an update: queue, don't recurse.
|
||||
(tp--queue-batch-update layer-name symbol where tp-text-affected)
|
||||
(let ((tp--reactive-updating t))
|
||||
;; Update computed properties for this layer
|
||||
(let ((updated-override
|
||||
(tp--update-layer-computed layer-name override-alist)))
|
||||
;; Update only the reactive properties in the layer definition.
|
||||
;; Buffer-local changes must not leak into the global definition;
|
||||
;; the buffer re-render below resolves against the buffer-local
|
||||
;; values instead.
|
||||
(when (and reactive-props (not (bufferp where)))
|
||||
(let ((resolved-props (tp--resolve-reactive-symbols
|
||||
reactive-props updated-override))
|
||||
(current-props (cdr (assoc layer-name tp-layer-alist))))
|
||||
(when current-props
|
||||
;; Deep merge the resolved reactive props into the current
|
||||
;; layer props to preserve nested plist values (like face)
|
||||
(tp--set-layer-props
|
||||
layer-name
|
||||
(tp--deep-merge-plist current-props resolved-props)))))
|
||||
;; Update text regions with this layer (or defer if batching)
|
||||
(if tp--batch-update-active
|
||||
;; Batching: defer the buffer update
|
||||
(progn
|
||||
(tp-debug-log " Deferring buffer update for %s (batch mode)"
|
||||
layer-name)
|
||||
(tp--queue-batch-update layer-name symbol where
|
||||
tp-text-affected))
|
||||
;; Normal: update immediately
|
||||
(tp-debug-log " Updating layer %s (tp-text affected: %s)"
|
||||
layer-name (if tp-text-affected "yes" "no"))
|
||||
(if tp-text-affected
|
||||
(tp--update-reactive-text layer-name where updated-override)
|
||||
(tp--update-layer-regions layer-name where updated-override)))))
|
||||
;; Re-renders queued by nested variable writes during this update are
|
||||
;; flushed now that the outermost update has finished.
|
||||
(unless tp--batch-update-active
|
||||
(when tp--batch-update-pending
|
||||
(tp--flush-batch-updates))))))
|
||||
(unwind-protect
|
||||
(let ((tp--reactive-updating t))
|
||||
;; Update computed properties for this layer
|
||||
(let ((updated-override
|
||||
(tp--update-layer-computed layer-name override-alist)))
|
||||
;; Update only the reactive properties in the layer definition.
|
||||
;; Buffer-local changes must not leak into the global definition;
|
||||
;; the buffer re-render below resolves against the buffer-local
|
||||
;; values instead.
|
||||
(when (and reactive-props (not (bufferp where)))
|
||||
(let ((resolved-props (tp--resolve-reactive-symbols
|
||||
reactive-props updated-override))
|
||||
(current-props (cdr (assoc layer-name tp-layer-alist))))
|
||||
(when current-props
|
||||
;; Deep merge the resolved reactive props into the current
|
||||
;; layer props to preserve nested plist values (like face)
|
||||
(tp--set-layer-props
|
||||
layer-name
|
||||
(tp--deep-merge-plist current-props resolved-props)))))
|
||||
;; Update text regions with this layer (or defer if batching)
|
||||
(if tp--batch-update-active
|
||||
;; Batching: defer the buffer update
|
||||
(progn
|
||||
(tp-debug-log " Deferring buffer update for %s (batch mode)"
|
||||
layer-name)
|
||||
(tp--queue-batch-update layer-name symbol where
|
||||
tp-text-affected))
|
||||
;; Normal: update immediately
|
||||
(tp-debug-log " Updating layer %s (tp-text affected: %s)"
|
||||
layer-name (if tp-text-affected "yes" "no"))
|
||||
(if tp-text-affected
|
||||
(tp--update-reactive-text layer-name where updated-override)
|
||||
(tp--update-layer-regions layer-name where updated-override)))))
|
||||
;; Re-renders queued by nested variable writes during this update
|
||||
;; are flushed now that the outermost update has finished. The
|
||||
;; flush runs under unwind-protect so an error escaping the
|
||||
;; re-render (for example from a modification hook) cannot strand
|
||||
;; queued entries in the global queue (ARCH-4); the reentrancy
|
||||
;; guard has been unbound by now, so the flush re-renders
|
||||
;; normally.
|
||||
(unless tp--batch-update-active
|
||||
(when tp--batch-update-pending
|
||||
(tp--flush-batch-updates)))))))
|
||||
|
||||
(defun tp--reactive-flush-entry (layer-name where tp-text-affected)
|
||||
"Re-render LAYER-NAME's regions in WHERE (or all buffers when nil).
|
||||
|
||||
Loading…
Reference in New Issue
Block a user