From e15a18d71893adbd10396b5a7cbd4efd083e65c1 Mon Sep 17 00:00:00 2001 From: Kinneyzhang Date: Mon, 27 Jul 2026 01:47:03 +0800 Subject: [PATCH] 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 --- tp-stack-tests.el | 123 ++++++++++++++++++++++++++++++++++++++++++++-- tp-stack.el | 122 ++++++++++++++++++++++++++++++--------------- 2 files changed, 203 insertions(+), 42 deletions(-) diff --git a/tp-stack-tests.el b/tp-stack-tests.el index 3114b5a..12bbe65 100644 --- a/tp-stack-tests.el +++ b/tp-stack-tests.el @@ -527,7 +527,10 @@ definitions cannot leak between tests." (should-not (tp-stack-tests--has-prop-p 1 'tp-layers))))) (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 (insert "abcdef") (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 '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-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 diff --git a/tp-stack.el b/tp-stack.el index 70a7014..d406d1b 100644 --- a/tp-stack.el +++ b/tp-stack.el @@ -812,36 +812,61 @@ Calling conventions: (tp-merge-layers STRING NEW-LAYER-NAME \\='(IDX1 LAYER-NAME1 IDX2 ...)) 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) (tp--parse-layer-args start-or-string (list end-or-name name-or-ids ids-or-object object) 2))) - (tp--stack-map-region - start end obj - (lambda (abs-start abs-end stack) - (let* ((layers-to-merge - (cl-loop for id in layer-ids - for found = (tp--get-layer-by-idx-or-name stack id) - when found collect found)) - ;; Sort by index (descending) to remove from end first - (sorted-layers (sort (copy-sequence layers-to-merge) - (lambda (a b) (> (car a) (car b)))))) - (when layers-to-merge - ;; Merge properties (earlier in list takes precedence) - (let ((merged-props (tp--merge-layer-props - layers-to-merge (list 'tp-name new-name))) - (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)))))) - nil)) + (let ((count 0)) + (tp--stack-map-region + start end obj + (lambda (abs-start abs-end stack) + (let* ((layers-to-merge + (cl-loop for id in layer-ids + for found = (tp--get-layer-by-idx-or-name stack id) + when found collect found)) + ;; Sort by index (descending) to remove from end first + (sorted-layers (sort (copy-sequence layers-to-merge) + (lambda (a b) (> (car a) (car b)))))) + (when layers-to-merge + ;; Merge properties (earlier in list takes precedence). + ;; Hidden layers contribute no props unless ALL matched + ;; layers are hidden, in which case the merged layer + ;; keeps their props but stays hidden itself. + (let* ((visible (seq-remove (lambda (found) + (tp--stack-hidden-p (cdr found))) + layers-to-merge)) + (merged-props + (if visible + (tp--merge-layer-props + visible (list 'tp-name new-name)) + (tp--merge-layer-props + 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) "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; 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) (tp--parse-layer-args start-or-string (list end-or-name name-or-object object) 1))) - (tp--stack-map-region - start end obj - (lambda (abs-start abs-end stack) - (when stack - (let ((merged-props (tp--merge-layer-props - (cl-loop for layer in stack - for i from 0 - collect (cons i layer)) - (when name (list 'tp-name name))))) - (set-text-properties abs-start abs-end merged-props obj) - (tp--stack-register-layers (list merged-props) obj))))) - nil)) + (let ((count 0)) + (tp--stack-map-region + start end obj + (lambda (abs-start abs-end stack) + (when stack + ;; Hidden layers are discarded; an all-hidden run flattens + ;; to bare text. + (let* ((visible (seq-remove #'tp--stack-hidden-p stack)) + (merged-props + (when visible + (tp--merge-layer-props + (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) "Add/merge properties to specified layers.