geekinney/tp
1
0
Fork 0
Code Issues Pull Requests Actions 2 Packages Projects Releases Wiki Activity

Insert before deleting in minimal-diff tp-text edits so markers track

Browse Source 
The minimal-diff reactive text edit deleted the differing span first
and inserted the replacement after.  Deleting collapsed an
insertion-type-nil marker sitting on the FIRST character of the
preserved suffix onto the edit start, so it ended up stranded before
the inserted text instead of on its (unchanged) character -
contradicting the documented "markers sitting in unchanged text keep
their positions".  The drift happened even for same-length
replacements ("0" -> "9").

tp--edit-region-minimal-diff now inserts the replacement at
edit-start first and then deletes the shifted old span
((delete-region (point) (+ (point) (- edit-end edit-start)))): the
insertion shifts the suffix-boundary marker right with its character
and the deletion pulls it back into place.  Markers whose characters
were deleted now end at the END of the edit rather than the start
(documented side effect, still inside the replacement span).

Point clamping semantics are preserved: the documented "point inside
the edited span ends up at the start of the edit" was previously an
artifact of save-excursion's own marker collapsing under
delete-then-insert, which the new edit order breaks.
tp--replace-reactive-text-in-buffer therefore owns point restoration
now - it remembers point in a marker, clamps it to the edit start
when an edit swallowed it, and restores it at the end - and
tp--update-reactive-text's save-excursion wrapper is removed (it
would have overridden the clamp with its drifted marker).  All four
shipped point/marker behavior tests pass unchanged.

New regressions port verify-txt1: suffix-start marker tracking for
grow, same-length and shrink edits (direct calls), the deleted-char
marker landing at the edit end, and a real setq-driven suffix-marker
case.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
This commit is contained in:
Kinneyzhang 2026-07-27 01:54:43 +08:00
parent 117c714b97
commit a0d0e71ff8
2 changed files with 173 additions and 53 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

79
tp-render-tests.el
View File

@ -41,6 +41,7 @@
(defvar tp-rt-r2-text nil) (defvar tp-rt-r2-text nil)
(defvar tp-rt-r2m-text nil) (defvar tp-rt-r2m-text nil)
(defvar tp-rt-r2n-text nil) (defvar tp-rt-r2n-text nil)
(defvar tp-rt-r2s-text nil)
(defvar tp-rt-r3a-color nil) (defvar tp-rt-r3a-color nil)
(defvar tp-rt-r3b-color nil) (defvar tp-rt-r3b-color nil)
(defvar tp-rt-r3c-color nil) (defvar tp-rt-r3c-color nil)
@ -564,6 +565,84 @@
(set-marker m-prefix nil) (set-marker m-prefix nil)
(set-marker m-suffix nil))))) (set-marker m-suffix nil)))))
;;; TXT-1: the suffix-boundary marker must track its character
(defun tp-rt--txt1-marker-after-edit (old new marker-offset)
"Run a minimal-diff replacement of OLD by NEW with a boundary marker.
Insert \"HEAD \" OLD \" TAIL\" in a temp buffer, tag OLD with a
tp-name, put an insertion-type-nil marker at OLD's start plus
MARKER-OFFSET, replace via `tp--replace-reactive-text-in-buffer' and
return (MARKER-POSITION CHAR-AT-MARKER ORIGINAL-CHAR)."
(with-temp-buffer
(insert "HEAD ")
(let ((m-start (point)))
(insert old " TAIL")
(put-text-property m-start (+ m-start (length old))
'tp-name 'tp-rt-txt1-layer)
(let* ((mpos (+ m-start marker-offset))
(mchar (char-after mpos))
(mk (copy-marker mpos)))
(tp--replace-reactive-text-in-buffer 'tp-rt-txt1-layer new nil)
(prog1 (list (marker-position mk) (char-after mk) mchar)
(set-marker mk nil))))))
(ert-deftest tp-render-test-minimal-diff-suffix-start-marker-tracks ()
"A marker on the FIRST character of the preserved suffix tracks it.
TXT-1: delete-then-insert collapsed such a marker onto the edit
start, stranding it before the inserted text; insert-then-delete
shifts it right with its character. Grow, same-length (the clearest
docstring violation) and shrink edits are all covered."
;; Grow: "0" -> "42"; marker on the space before "items" (offset 8).
(pcase-let ((`(,pos ,got ,want)
(tp-rt--txt1-marker-after-edit
"count: 0 items" "count: 42 items" 8)))
(should (eq got want))
(should (= pos 15))) ; 14 shifted right by 1
;; Same length: "0" -> "9"; the marker's correct position is
;; numerically unchanged.
(pcase-let ((`(,pos ,got ,want)
(tp-rt--txt1-marker-after-edit
"count: 0 items" "count: 9 items" 8)))
(should (eq got want))
(should (= pos 14)))
;; Shrink: "42" -> "0".
(pcase-let ((`(,pos ,got ,want)
(tp-rt--txt1-marker-after-edit
"count: 42 items" "count: 0 items" 9)))
(should (eq got want))
(should (= pos 14))))
(ert-deftest tp-render-test-minimal-diff-deleted-char-marker-at-edit-end ()
"A marker whose character was deleted ends at the END of the edit.
The documented side effect of inserting before deleting; previously
such markers collapsed to the edit start. Either way they stay
inside the replacement span."
;; "100" -> "42": marker on the middle "0" (strictly inside the
;; edited span) ends after the inserted "42".
(pcase-let ((`(,pos ,_got ,_want)
(tp-rt--txt1-marker-after-edit
"count: 100 items" "count: 42 items" 8)))
;; Edit span starts at buffer position 13 ("100"), insert "42":
;; the marker lands at the end of the inserted text.
(should (= pos 15))))
(ert-deftest tp-render-test-minimal-diff-suffix-marker-real-path ()
"The suffix-start marker tracks through a real setq-driven update."
(tp-rt-with-cleanup (tp-rt-r2s-layer) (tp-rt-r2s-text)
(setq tp-rt-r2s-text "count: 0 items")
(define-tp tp-rt-r2s-layer () '(tp-text $tp-rt-r2s-text))
(with-temp-buffer
(insert "count: 0 items")
(tp-set 1 15 'tp-rt-r2s-layer)
(let ((m (copy-marker 9))) ; the space before "items"
(setq tp-rt-r2s-text "count: 42 items")
(should (equal (buffer-substring-no-properties (point-min)
(point-max))
"count: 42 items"))
(should (eq (char-after m) ?\s))
(should (= (marker-position m) 10))
(set-marker m nil)))))
(ert-deftest tp-render-test-minimal-diff-identical-update-is-noop () (ert-deftest tp-render-test-minimal-diff-identical-update-is-noop ()
"An identical-text reactive replacement leaves the buffer unmodified." "An identical-text reactive replacement leaves the buffer unmodified."
(tp-rt-with-cleanup (tp-rt-r2n-layer) (tp-rt-r2n-text) (tp-rt-with-cleanup (tp-rt-r2n-layer) (tp-rt-r2n-text)

147
tp-render.el
View File

@ -346,23 +346,32 @@ it will be applied to the text before updating."
(tp--tp-text-transform layer-name raw-text) (tp--tp-text-transform layer-name raw-text)
raw-text))) raw-text)))
(when (and new-text (stringp new-text)) (when (and new-text (stringp new-text))
(save-excursion ;; No save-excursion here: the replace function
(tp--replace-reactive-text-in-buffer ;; owns point restoration (its clamping semantics
layer-name new-text props))))))))) ;; would be overridden by save-excursion's own
;; drifting marker).
(tp--replace-reactive-text-in-buffer
layer-name new-text props))))))))
(tp--map-layer-buffers layer-name where update-buffer))) (tp--map-layer-buffers layer-name where update-buffer)))
(defun tp--edit-region-minimal-diff (m-start m-end plain-text skip-props) (defun tp--edit-region-minimal-diff (m-start m-end plain-text skip-props)
"Make [M-START, M-END) of the current buffer read PLAIN-TEXT. "Make [M-START, M-END) of the current buffer read PLAIN-TEXT.
Only the differing span of the region is edited: the common prefix Only the differing span of the region is edited: the common prefix
and suffix of the old and new text are left untouched, so point and and suffix of the old and new text are left untouched. The
markers sitting in unchanged text keep their positions (point inside replacement is inserted BEFORE the old span is deleted, so markers
the edited span ends up at the start of the edit). Does nothing when sitting in unchanged text keep tracking their characters - including
the region already reads PLAIN-TEXT, so an identical-text update does a marker at the first character of the preserved suffix, which the
not mark the buffer as modified. old delete-then-insert order collapsed onto the edit start (TXT-1).
Markers whose characters were deleted end up at the end of the edit.
Does nothing when the region already reads PLAIN-TEXT, so an
identical-text update does not mark the buffer as modified.
Properties present at M-START whose keys the plist SKIP-PROPS does Properties present at M-START whose keys the plist SKIP-PROPS does
not contain are re-applied over the edited span (a nil SKIP-PROPS not contain are re-applied over the edited span (a nil SKIP-PROPS
carries every existing property); the untouched prefix and suffix carries every existing property); the untouched prefix and suffix
keep their own properties as is." keep their own properties as is.
Returns the cons (EDIT-START . EDIT-END) of the replaced span in
PRE-edit coordinates - the caller uses it to clamp a remembered
point that sat inside the edit - or nil when nothing was edited."
(let ((old-text (buffer-substring-no-properties m-start m-end))) (let ((old-text (buffer-substring-no-properties m-start m-end)))
(unless (equal old-text plain-text) (unless (equal old-text plain-text)
;; Text content differs: trim the common prefix and suffix and ;; Text content differs: trim the common prefix and suffix and
@ -384,11 +393,14 @@ keep their own properties as is."
(edit-end (- m-end suffix)) (edit-end (- m-end suffix))
(insert-text (substring plain-text prefix (- new-len suffix))) (insert-text (substring plain-text prefix (- new-len suffix)))
(existing-props (text-properties-at m-start))) (existing-props (text-properties-at m-start)))
(when (< edit-start edit-end) ;; Insert first, then delete the (shifted) old span: an
(delete-region edit-start edit-end)) ;; insertion-type-nil marker at the start of the preserved
(when (> (length insert-text) 0) ;; suffix sits strictly after EDIT-START, so the insertion
(goto-char edit-start) ;; shifts it right with its character, and the deletion of
(insert insert-text)) ;; the old span just before it shifts it back into place.
(goto-char edit-start)
(insert insert-text)
(delete-region (point) (+ (point) (- edit-end edit-start)))
;; Carry over existing properties whose keys SKIP-PROPS does ;; Carry over existing properties whose keys SKIP-PROPS does
;; not name onto the newly inserted span; the untouched ;; not name onto the newly inserted span; the untouched
;; prefix and suffix keep their own properties as is. ;; prefix and suffix keep their own properties as is.
@ -396,7 +408,8 @@ keep their own properties as is."
(cl-loop for (key val) on existing-props by #'cddr (cl-loop for (key val) on existing-props by #'cddr
do (unless (plist-member skip-props key) do (unless (plist-member skip-props key)
(put-text-property edit-start mid-end key (put-text-property edit-start mid-end key
val))))))))) val))))
(cons edit-start edit-end))))))
(defun tp--pos-holds-layer-in-storage-only-p (pos layer-name) (defun tp--pos-holds-layer-in-storage-only-p (pos layer-name)
"Return non-nil when POS holds LAYER-NAME only inside `tp-layers'. "Return non-nil when POS holds LAYER-NAME only inside `tp-layers'.
@ -433,44 +446,72 @@ properties, never text), so the model value still replaces the text
there, but the layer's props are not applied directly; instead its there, but the layer's props are not applied directly; instead its
stored stack entry, including the refreshed `tp-text', is written stored stack entry, including the refreshed `tp-text', is written
through, so `tp-show-layer' or a reveal by a later stack operation through, so `tp-show-layer' or a reveal by a later stack operation
renders current values." renders current values.
(goto-char (point-min)) This function owns point restoration (callers must not wrap it in
(let ((match (text-property-search-forward 'tp-name layer-name t)) `save-excursion', whose own marker would drift): point outside the
(plain-text (substring-no-properties new-text))) edits keeps tracking its character, and point inside an edited span
;; Pass 1: regions where the layer is the rendered top layer is clamped to the start of that edit."
;; (direct `tp-name'). (let ((plain-text (substring-no-properties new-text))
(while match ;; Remember where the user's point was; the marker tracks all
(let* ((m-start (prop-match-beginning match)) ;; edits, and edits that swallow point clamp it explicitly.
(m-end (prop-match-end match))) (orig-point (copy-marker (point))))
(tp--edit-region-minimal-diff m-start m-end plain-text props) (unwind-protect
;; Apply the layer's props, merged per embedded interval of NEW-TEXT. (cl-flet ((edit-tracking-point (m-start m-end skip-props)
;; Keys are replaced (not accumulated); unrelated keys are untouched. ;; Run the minimal-diff edit; when the remembered
(tp--apply-reactive-text-props new-text props m-start) ;; point sat inside the replaced span, clamp it to
;; Continue searching after the fully updated region: a preserved ;; the start of the edit (the documented
;; suffix still carries the layer's `tp-name', and restarting the ;; behavior).
;; search inside it would re-match this region. (let* ((was (marker-position orig-point))
(goto-char (+ m-start (length plain-text)))) (span (tp--edit-region-minimal-diff
(setq match (text-property-search-forward 'tp-name layer-name t))) m-start m-end plain-text skip-props)))
;; Pass 2: regions where the layer sits only inside stack storage. (when (and span
;; Replace their text too, carrying ALL existing properties (the (>= was (car span))
;; visible top layer's render cache and the `tp-layers' storage) (< was (cdr span)))
;; over the edited span; the hidden/buried layer's own props are (set-marker orig-point (car span))))))
;; not applied directly. (goto-char (point-min))
(let ((pos (point-min))) ;; Pass 1: regions where the layer is the rendered top layer
(while (< pos (point-max)) ;; (direct `tp-name').
(if (tp--pos-holds-layer-in-storage-only-p pos layer-name) (let ((match (text-property-search-forward 'tp-name
(let ((region-end pos)) layer-name t)))
(while (and (< region-end (point-max)) (while match
(tp--pos-holds-layer-in-storage-only-p (let* ((m-start (prop-match-beginning match))
region-end layer-name)) (m-end (prop-match-end match)))
(setq region-end (or (next-property-change region-end) (edit-tracking-point m-start m-end props)
(point-max)))) ;; Apply the layer's props, merged per embedded interval
(tp--edit-region-minimal-diff pos region-end plain-text nil) ;; of NEW-TEXT. Keys are replaced (not accumulated);
(setq pos (+ pos (length plain-text)))) ;; unrelated keys are untouched.
(setq pos (or (next-property-change pos) (point-max)))))) (tp--apply-reactive-text-props new-text props m-start)
;; Write the updated props - including the refreshed `tp-text' - ;; Continue searching after the fully updated region: a
;; through to the layer's entries in stack storage (HID-1). ;; preserved suffix still carries the layer's `tp-name',
(tp--write-layer-through-stack-storage layer-name props))) ;; and restarting the search inside it would re-match
;; this region.
(goto-char (+ m-start (length plain-text))))
(setq match (text-property-search-forward 'tp-name
layer-name t))))
;; Pass 2: regions where the layer sits only inside stack
;; storage. Replace their text too, carrying ALL existing
;; properties (the visible top layer's render cache and the
;; `tp-layers' storage) over the edited span; the
;; hidden/buried layer's own props are not applied directly.
(let ((pos (point-min)))
(while (< pos (point-max))
(if (tp--pos-holds-layer-in-storage-only-p pos layer-name)
(let ((region-end pos))
(while (and (< region-end (point-max))
(tp--pos-holds-layer-in-storage-only-p
region-end layer-name))
(setq region-end (or (next-property-change
region-end)
(point-max))))
(edit-tracking-point pos region-end nil)
(setq pos (+ pos (length plain-text))))
(setq pos (or (next-property-change pos) (point-max))))))
;; Write the updated props - including the refreshed
;; `tp-text' - through to the layer's entries in stack
;; storage (HID-1).
(tp--write-layer-through-stack-storage layer-name props))
(goto-char orig-point)
(set-marker orig-point nil))))
(defun tp--tp-text-replace (start end final-text result-props object preserve-props) (defun tp--tp-text-replace (start end final-text result-props object preserve-props)
"Replace [START, END) of OBJECT with FINAL-TEXT, handling props. "Replace [START, END) of OBJECT with FINAL-TEXT, handling props.