Flatten/merge no longer render hidden layers; both return run counts
tp-hide-layer's contract says a hidden layer "no longer renders", but tp-flatten-layers merged the whole stack unfiltered and tp-merge-layers merged every listed layer unfiltered (only the tp-hidden bookkeeping flag was stripped), so flattening or merging a stack with a hidden layer silently un-hid it: the hidden layer's props became the visible rendering (green -> red in the review probe). The shipped flatten test asserted only flag absence, missing the rendering flip. New semantics, stated in both docstrings: - tp-flatten-layers discards hidden layers (image-editor flatten): only visible layers' props merge; a run whose every layer is hidden flattens to bare text, consistent with all-hidden rendering. - tp-merge-layers merges hidden matched layers away but excludes their props, so a merge can never render what was hidden. When ALL matched layers are hidden the merged layer keeps their merged props but carries tp-hidden itself - data preserved, nothing un-hidden, tp-show-layer reveals it. API-RET-01 slice: both functions now return the number of modified runs, counting exactly like tp-delete-layer (their previous hardcoded nil return was undocumented, so no documented behavior changes). Tests: the flatten-drops-tp-hidden-flag test now asserts the rendered face; new tests port the hid2-probe scenarios (flatten with hidden top, all-hidden flatten to bare text, merge excluding hidden props, all-hidden merge staying hidden and revealable) plus count-return coverage for both functions. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
This commit is contained in:
parent
b49e2740e8
commit
e15a18d718
@ -527,7 +527,10 @@ definitions cannot leak between tests."
|
|||||||
(should-not (tp-stack-tests--has-prop-p 1 'tp-layers)))))
|
(should-not (tp-stack-tests--has-prop-p 1 'tp-layers)))))
|
||||||
|
|
||||||
(ert-deftest tp-stack-test-flatten-drops-tp-hidden-flag ()
|
(ert-deftest tp-stack-test-flatten-drops-tp-hidden-flag ()
|
||||||
"Flattening a stack with a hidden layer never leaks the tp-hidden flag."
|
"Flattening a stack with a hidden layer never leaks the tp-hidden flag.
|
||||||
|
HID-2: the hidden layer's props are discarded entirely, so the
|
||||||
|
flattened result renders the visible layer's face, not the hidden
|
||||||
|
one's."
|
||||||
(tp-stack-tests--with-env
|
(tp-stack-tests--with-env
|
||||||
(insert "abcdef")
|
(insert "abcdef")
|
||||||
(define-tp lower () '(face bold))
|
(define-tp lower () '(face bold))
|
||||||
@ -535,10 +538,124 @@ definitions cannot leak between tests."
|
|||||||
(tp-push-layer 1 6 'lower)
|
(tp-push-layer 1 6 'lower)
|
||||||
(tp-push-layer 1 6 'upper)
|
(tp-push-layer 1 6 'upper)
|
||||||
(tp-hide-layer 1 6 'upper)
|
(tp-hide-layer 1 6 'upper)
|
||||||
(tp-flatten-layers 1 6 'flat)
|
(should (= (tp-flatten-layers 1 6 'flat) 1))
|
||||||
(should (eq (get-text-property 1 'tp-name) 'flat))
|
(should (eq (get-text-property 1 'tp-name) 'flat))
|
||||||
(should-not (tp-stack-tests--has-prop-p 1 'tp-hidden))
|
(should-not (tp-stack-tests--has-prop-p 1 'tp-hidden))
|
||||||
(should-not (tp-stack-tests--has-prop-p 1 'tp-layers))))
|
(should-not (tp-stack-tests--has-prop-p 1 'tp-layers))
|
||||||
|
;; The visible layer's face renders; the hidden italic is gone.
|
||||||
|
(should (eq (get-text-property 1 'face) 'bold))))
|
||||||
|
|
||||||
|
;;; HID-2: flatten/merge must not render hidden layers' properties
|
||||||
|
|
||||||
|
(ert-deftest tp-stack-test-flatten-discards-hidden-layer-props ()
|
||||||
|
"Flatten discards a hidden layer's props instead of rendering them.
|
||||||
|
Probe scenario A: red (hidden, with help-echo) over green over blue
|
||||||
|
\(with mouse-face); the flattened result must show green and keep
|
||||||
|
blue's mouse-face, with no trace of the hidden red layer."
|
||||||
|
(tp-stack-tests--with-env
|
||||||
|
(insert "abcdef")
|
||||||
|
(define-tp tp-st-h2-red () '(face (:foreground "red") help-echo "red"))
|
||||||
|
(define-tp tp-st-h2-green () '(face (:foreground "green")))
|
||||||
|
(define-tp tp-st-h2-blue () '(face (:foreground "blue")
|
||||||
|
mouse-face highlight))
|
||||||
|
(tp-push-layer 1 6 'tp-st-h2-blue)
|
||||||
|
(tp-push-layer 1 6 'tp-st-h2-green)
|
||||||
|
(tp-push-layer 1 6 'tp-st-h2-red) ; top->bottom: red green blue
|
||||||
|
(tp-hide-layer 1 6 'tp-st-h2-red)
|
||||||
|
(should (equal (get-text-property 1 'face) '(:foreground "green")))
|
||||||
|
(should (= (tp-flatten-layers 1 6 'flat) 1))
|
||||||
|
(should (equal (get-text-property 1 'face) '(:foreground "green")))
|
||||||
|
(should-not (tp-stack-tests--has-prop-p 1 'help-echo))
|
||||||
|
(should (eq (get-text-property 1 'mouse-face) 'highlight))))
|
||||||
|
|
||||||
|
(ert-deftest tp-stack-test-flatten-all-hidden-yields-bare-text ()
|
||||||
|
"Flattening a run whose every layer is hidden clears all properties.
|
||||||
|
Consistent with the all-hidden rendering of `tp-hide-layer'; the run
|
||||||
|
still counts as modified in the returned count."
|
||||||
|
(tp-stack-tests--with-env
|
||||||
|
(insert "abcdef")
|
||||||
|
(define-tp tp-st-h2a-one () '(face bold))
|
||||||
|
(define-tp tp-st-h2a-two () '(face italic))
|
||||||
|
(tp-push-layer 1 6 'tp-st-h2a-one)
|
||||||
|
(tp-push-layer 1 6 'tp-st-h2a-two)
|
||||||
|
(tp-hide-layer 1 6 'tp-st-h2a-one)
|
||||||
|
(tp-hide-layer 1 6 'tp-st-h2a-two)
|
||||||
|
(should (= (tp-flatten-layers 1 6 'flat) 1))
|
||||||
|
(should (null (text-properties-at 1)))))
|
||||||
|
|
||||||
|
(ert-deftest tp-stack-test-merge-excludes-hidden-layer-props ()
|
||||||
|
"Merging a hidden layer with a visible one excludes the hidden props.
|
||||||
|
Probe scenario B: merging hidden red with visible green removes both
|
||||||
|
from the stack but the merged layer renders green - a merge must
|
||||||
|
never un-hide what `tp-hide-layer' hid."
|
||||||
|
(tp-stack-tests--with-env
|
||||||
|
(insert "abcdef")
|
||||||
|
(define-tp tp-st-h2b-red () '(face (:foreground "red")))
|
||||||
|
(define-tp tp-st-h2b-green () '(face (:foreground "green")))
|
||||||
|
(define-tp tp-st-h2b-blue () '(face (:foreground "blue")))
|
||||||
|
(tp-push-layer 1 6 'tp-st-h2b-blue)
|
||||||
|
(tp-push-layer 1 6 'tp-st-h2b-green)
|
||||||
|
(tp-push-layer 1 6 'tp-st-h2b-red)
|
||||||
|
(tp-hide-layer 1 6 'tp-st-h2b-red)
|
||||||
|
(should (= (tp-merge-layers 1 6 'merged '(tp-st-h2b-red tp-st-h2b-green))
|
||||||
|
1))
|
||||||
|
(should (equal (get-text-property 1 'face) '(:foreground "green")))
|
||||||
|
(should (eq (get-text-property 1 'tp-name) 'merged))
|
||||||
|
(should (equal (mapcar #'car (tp-layer-stack-at 1))
|
||||||
|
'(merged tp-st-h2b-blue)))))
|
||||||
|
|
||||||
|
(ert-deftest tp-stack-test-merge-all-hidden-stays-hidden ()
|
||||||
|
"Merging only hidden layers produces a hidden merged layer.
|
||||||
|
The merged layer keeps the hidden layers' merged props (data is
|
||||||
|
preserved) but carries tp-hidden itself, so nothing starts rendering;
|
||||||
|
`tp-show-layer' can reveal it later."
|
||||||
|
(tp-stack-tests--with-env
|
||||||
|
(insert "abcdef")
|
||||||
|
(define-tp tp-st-h2c-red () '(face (:foreground "red")))
|
||||||
|
(define-tp tp-st-h2c-green () '(face (:foreground "green")))
|
||||||
|
(define-tp tp-st-h2c-blue () '(face (:foreground "blue")))
|
||||||
|
(tp-push-layer 1 6 'tp-st-h2c-blue)
|
||||||
|
(tp-push-layer 1 6 'tp-st-h2c-green)
|
||||||
|
(tp-push-layer 1 6 'tp-st-h2c-red)
|
||||||
|
(tp-hide-layer 1 6 'tp-st-h2c-red)
|
||||||
|
(tp-hide-layer 1 6 'tp-st-h2c-green)
|
||||||
|
(should (= (tp-merge-layers 1 6 'merged
|
||||||
|
'(tp-st-h2c-red tp-st-h2c-green))
|
||||||
|
1))
|
||||||
|
;; The merged layer does not render: blue stays visible.
|
||||||
|
(should (equal (get-text-property 1 'face) '(:foreground "blue")))
|
||||||
|
;; It is present, hidden, and carries the merged (red-wins) props.
|
||||||
|
(let ((entry (assq 'merged (tp-layer-stack-at 1))))
|
||||||
|
(should entry)
|
||||||
|
(should (eq (plist-get (cdr entry) 'tp-hidden) t))
|
||||||
|
(should (equal (plist-get (cdr entry) 'face) '(:foreground "red"))))
|
||||||
|
;; Showing the merged layer renders it.
|
||||||
|
(tp-show-layer 1 6 'merged)
|
||||||
|
(should (equal (get-text-property 1 'face) '(:foreground "red")))))
|
||||||
|
|
||||||
|
;;; HID2-RET: merge/flatten return modified-run counts
|
||||||
|
|
||||||
|
(ert-deftest tp-stack-test-merge-and-flatten-return-counts ()
|
||||||
|
"tp-merge-layers / tp-flatten-layers return modified-run counts.
|
||||||
|
Counting matches `tp-delete-layer': one per rewritten run, 0 when
|
||||||
|
nothing matched."
|
||||||
|
(tp-stack-tests--with-env
|
||||||
|
(insert "abcdefghij")
|
||||||
|
(define-tp tp-st-ret-a () '(face bold))
|
||||||
|
(define-tp tp-st-ret-b () '(face italic))
|
||||||
|
;; Two separate runs with different stacks.
|
||||||
|
(tp-push-layer 1 4 'tp-st-ret-a)
|
||||||
|
(tp-push-layer 1 4 'tp-st-ret-b)
|
||||||
|
(tp-push-layer 5 8 'tp-st-ret-a)
|
||||||
|
;; Merge matches both layers in run 1, only one in run 2: both
|
||||||
|
;; runs are rewritten.
|
||||||
|
(should (= (tp-merge-layers 1 8 'm '(tp-st-ret-a tp-st-ret-b)) 2))
|
||||||
|
;; Nothing matches on bare text.
|
||||||
|
(should (= (tp-merge-layers 8 11 'm2 '(tp-st-ret-a)) 0))
|
||||||
|
;; Flatten counts every run that had layers ([1,4) and [5,8) are
|
||||||
|
;; separated by bare text); bare text does not count.
|
||||||
|
(should (= (tp-flatten-layers 1 8 'flat) 2))
|
||||||
|
(should (= (tp-flatten-layers 8 11 'flat2) 0))))
|
||||||
|
|
||||||
;;; 0.3.0 S2: tp-lower-layer and extended tp-rotate-layer
|
;;; 0.3.0 S2: tp-lower-layer and extended tp-rotate-layer
|
||||||
|
|
||||||
|
|||||||
122
tp-stack.el
122
tp-stack.el
@ -812,36 +812,61 @@ Calling conventions:
|
|||||||
(tp-merge-layers STRING NEW-LAYER-NAME \\='(IDX1 LAYER-NAME1 IDX2 ...))
|
(tp-merge-layers STRING NEW-LAYER-NAME \\='(IDX1 LAYER-NAME1 IDX2 ...))
|
||||||
|
|
||||||
Earlier layers in the list take precedence; a property explicitly set
|
Earlier layers in the list take precedence; a property explicitly set
|
||||||
to nil in a higher-precedence layer stays nil in the merged layer."
|
to nil in a higher-precedence layer stays nil in the merged layer.
|
||||||
|
|
||||||
|
Hidden matched layers (see `tp-hide-layer') are merged away with the
|
||||||
|
rest but contribute NO properties to the merged layer, so a merge can
|
||||||
|
never render what was hidden. When EVERY matched layer of a run is
|
||||||
|
hidden, the merged layer keeps their merged properties but carries
|
||||||
|
the `tp-hidden' flag itself: the data is preserved without un-hiding
|
||||||
|
anything, and `tp-show-layer' on the merged layer renders it.
|
||||||
|
|
||||||
|
Returns the number of property runs modified, counting like
|
||||||
|
`tp-delete-layer': a run counts when at least one listed layer
|
||||||
|
matched and the merge rewrote it, and 0 means nothing matched at
|
||||||
|
all."
|
||||||
(pcase-let ((`(,start ,end ,obj ,new-name ,layer-ids)
|
(pcase-let ((`(,start ,end ,obj ,new-name ,layer-ids)
|
||||||
(tp--parse-layer-args
|
(tp--parse-layer-args
|
||||||
start-or-string
|
start-or-string
|
||||||
(list end-or-name name-or-ids ids-or-object object) 2)))
|
(list end-or-name name-or-ids ids-or-object object) 2)))
|
||||||
(tp--stack-map-region
|
(let ((count 0))
|
||||||
start end obj
|
(tp--stack-map-region
|
||||||
(lambda (abs-start abs-end stack)
|
start end obj
|
||||||
(let* ((layers-to-merge
|
(lambda (abs-start abs-end stack)
|
||||||
(cl-loop for id in layer-ids
|
(let* ((layers-to-merge
|
||||||
for found = (tp--get-layer-by-idx-or-name stack id)
|
(cl-loop for id in layer-ids
|
||||||
when found collect found))
|
for found = (tp--get-layer-by-idx-or-name stack id)
|
||||||
;; Sort by index (descending) to remove from end first
|
when found collect found))
|
||||||
(sorted-layers (sort (copy-sequence layers-to-merge)
|
;; Sort by index (descending) to remove from end first
|
||||||
(lambda (a b) (> (car a) (car b))))))
|
(sorted-layers (sort (copy-sequence layers-to-merge)
|
||||||
(when layers-to-merge
|
(lambda (a b) (> (car a) (car b))))))
|
||||||
;; Merge properties (earlier in list takes precedence)
|
(when layers-to-merge
|
||||||
(let ((merged-props (tp--merge-layer-props
|
;; Merge properties (earlier in list takes precedence).
|
||||||
layers-to-merge (list 'tp-name new-name)))
|
;; Hidden layers contribute no props unless ALL matched
|
||||||
(new-stack stack))
|
;; layers are hidden, in which case the merged layer
|
||||||
;; Remove old layers from stack
|
;; keeps their props but stays hidden itself.
|
||||||
(dolist (idx (mapcar #'car sorted-layers))
|
(let* ((visible (seq-remove (lambda (found)
|
||||||
(setq new-stack (-remove-at idx new-stack)))
|
(tp--stack-hidden-p (cdr found)))
|
||||||
;; Add merged layer at top
|
layers-to-merge))
|
||||||
(setq new-stack (cons merged-props new-stack))
|
(merged-props
|
||||||
(set-text-properties abs-start abs-end
|
(if visible
|
||||||
(tp--stack-build-props new-stack)
|
(tp--merge-layer-props
|
||||||
obj)
|
visible (list 'tp-name new-name))
|
||||||
(tp--stack-register-layers new-stack obj))))))
|
(tp--merge-layer-props
|
||||||
nil))
|
layers-to-merge
|
||||||
|
(list 'tp-name new-name 'tp-hidden t))))
|
||||||
|
(new-stack stack))
|
||||||
|
;; Remove old layers from stack
|
||||||
|
(dolist (idx (mapcar #'car sorted-layers))
|
||||||
|
(setq new-stack (-remove-at idx new-stack)))
|
||||||
|
;; Add merged layer at top
|
||||||
|
(setq new-stack (cons merged-props new-stack))
|
||||||
|
(set-text-properties abs-start abs-end
|
||||||
|
(tp--stack-build-props new-stack)
|
||||||
|
obj)
|
||||||
|
(tp--stack-register-layers new-stack obj)
|
||||||
|
(setq count (1+ count)))))))
|
||||||
|
count)))
|
||||||
|
|
||||||
(defun tp-flatten-layers (start-or-string &optional end-or-name name-or-object object)
|
(defun tp-flatten-layers (start-or-string &optional end-or-name name-or-object object)
|
||||||
"Flatten all layers into a single layer.
|
"Flatten all layers into a single layer.
|
||||||
@ -855,23 +880,42 @@ Calling conventions:
|
|||||||
|
|
||||||
NAME can be nil for an unnamed layer. Higher layers take precedence;
|
NAME can be nil for an unnamed layer. Higher layers take precedence;
|
||||||
a property explicitly set to nil in a higher layer stays nil in the
|
a property explicitly set to nil in a higher layer stays nil in the
|
||||||
flattened result."
|
flattened result.
|
||||||
|
|
||||||
|
Hidden layers (see `tp-hide-layer') are DISCARDED, mirroring
|
||||||
|
image-editor flatten semantics: only the visible layers' properties
|
||||||
|
merge into the flattened result, so flattening can never render what
|
||||||
|
was hidden. When EVERY layer of a run is hidden, the run's
|
||||||
|
properties are cleared entirely (bare text), consistent with the
|
||||||
|
all-hidden rendering of `tp-hide-layer'.
|
||||||
|
|
||||||
|
Returns the number of property runs modified, counting like
|
||||||
|
`tp-delete-layer': every run that had layers to flatten counts, and
|
||||||
|
0 means no run in the region had any layers."
|
||||||
(pcase-let ((`(,start ,end ,obj ,name)
|
(pcase-let ((`(,start ,end ,obj ,name)
|
||||||
(tp--parse-layer-args
|
(tp--parse-layer-args
|
||||||
start-or-string
|
start-or-string
|
||||||
(list end-or-name name-or-object object) 1)))
|
(list end-or-name name-or-object object) 1)))
|
||||||
(tp--stack-map-region
|
(let ((count 0))
|
||||||
start end obj
|
(tp--stack-map-region
|
||||||
(lambda (abs-start abs-end stack)
|
start end obj
|
||||||
(when stack
|
(lambda (abs-start abs-end stack)
|
||||||
(let ((merged-props (tp--merge-layer-props
|
(when stack
|
||||||
(cl-loop for layer in stack
|
;; Hidden layers are discarded; an all-hidden run flattens
|
||||||
for i from 0
|
;; to bare text.
|
||||||
collect (cons i layer))
|
(let* ((visible (seq-remove #'tp--stack-hidden-p stack))
|
||||||
(when name (list 'tp-name name)))))
|
(merged-props
|
||||||
(set-text-properties abs-start abs-end merged-props obj)
|
(when visible
|
||||||
(tp--stack-register-layers (list merged-props) obj)))))
|
(tp--merge-layer-props
|
||||||
nil))
|
(cl-loop for layer in visible
|
||||||
|
for i from 0
|
||||||
|
collect (cons i layer))
|
||||||
|
(when name (list 'tp-name name))))))
|
||||||
|
(set-text-properties abs-start abs-end merged-props obj)
|
||||||
|
(when merged-props
|
||||||
|
(tp--stack-register-layers (list merged-props) obj))
|
||||||
|
(setq count (1+ count))))))
|
||||||
|
count)))
|
||||||
|
|
||||||
(defun tp-add-to-layers (idx-or-layer-name-list start-or-string &optional end-or-plist plist-or-object &rest rest)
|
(defun tp-add-to-layers (idx-or-layer-name-list start-or-string &optional end-or-plist plist-or-object &rest rest)
|
||||||
"Add/merge properties to specified layers.
|
"Add/merge properties to specified layers.
|
||||||
|
|||||||
Loading…
Reference in New Issue
Block a user