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

feat: add recursive resolution for nested custom layer properties

Browse Source 
When a define-tp layer returns a plist containing other custom layer names,
those are now recursively expanded to their built-in text properties.

This fixes the issue where tp-button using tp-palette internally would
leave tp-palette as a text property instead of resolving it to the
actual face properties.

Changes:
- Added tp--plist-has-layer-key-p helper function
- Updated tp--expand-layer-in-plist to recursively expand layer props
- Updated tp-layer-props to expand nested layers in returned plist
- Updated tp-layer-props-with-arg to expand nested layers in returned plist
- Added 3 tests for nested layer resolution

Co-authored-by: Kinneyzhang <38454496+Kinneyzhang@users.noreply.github.com>
This commit is contained in:
copilot-swe-agent[bot] 2025-12-31 08:15:32 +00:00
parent d824de77ee
commit 3410e28dac
2 changed files with 106 additions and 5 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

80
tp-tests.el
View File

@ -3622,5 +3622,85 @@ When using tp-set (direct property setting), tp-name is NOT added."
;; Valid parameterized layer should work
(should (define-tp tp-test-param-valid (value) `(face (:height ,value))))))
;;; ============================================================
;;; Nested Layer Resolution Tests
;;; ============================================================
(ert-deftest tp-test-nested-layer-resolution ()
"Test that nested custom layers are resolved to built-in properties.
When a layer's body returns a plist containing other custom layer names,
those should be recursively expanded to their built-in properties."
(tp-test-with-temp-buffer
;; Define a base layer that returns built-in properties
(define-tp tp-test-base-layer (color)
`(face (:foreground ,color :background "white")))
;; Define a wrapper layer that uses the base layer
(define-tp tp-test-wrapper-layer (plist)
(let ((color (plist-get plist :color)))
`(tp-test-base-layer ,color
help-echo "wrapper")))
;; Use the wrapper layer
(let ((result (tp-set "test" 'tp-test-wrapper-layer '(:color "red"))))
;; The face property should be resolved from tp-test-base-layer
(should (equal (plist-get (get-text-property 0 'face result) :foreground) "red"))
(should (equal (plist-get (get-text-property 0 'face result) :background) "white"))
;; help-echo should also be present
(should (equal (get-text-property 0 'help-echo result) "wrapper"))
;; tp-test-base-layer should NOT be present as a property
(should (null (get-text-property 0 'tp-test-base-layer result))))))
(ert-deftest tp-test-nested-layer-resolution-with-tp-palette ()
"Test nested layer resolution with tp-palette and tp-button pattern.
This tests the exact use case from the issue: tp-button uses tp-palette
internally, and the final result should have face properties, not tp-palette."
(tp-test-with-temp-buffer
;; Re-define tp-palette and tp-button since tp-test-with-temp-buffer resets tp-layer-alist
(define-tp tp-palette (symbol)
(let ((palette (intern (concat "tp-palette-"
(symbol-name symbol)))))
`(face ( :foreground ,(tp-palette-fg-color palette)
:background ,(tp-palette-bg-color palette)
:box (:color ,(tp-palette-border-color palette))))))
(define-tp tp-button (plist)
(let ((palette (plist-get plist :palette))
(action (plist-get plist :action)))
`( tp-palette ,palette
keymap ,(let ((keymap (make-sparse-keymap)))
(define-key keymap (kbd "<RET>") action)
(define-key keymap [mouse-1] action)
keymap))))
;; Test that using tp-button resolves tp-palette to face properties
(let ((result (tp-set "emacs" 'tp-button '(:palette org-code))))
;; The face property should be resolved from tp-palette
(let ((face-prop (get-text-property 0 'face result)))
(should face-prop)
(should (plist-get face-prop :foreground))
(should (plist-get face-prop :background))
(should (plist-get face-prop :box)))
;; keymap should also be present
(should (get-text-property 0 'keymap result))
;; tp-palette should NOT be present as a property
(should (null (get-text-property 0 'tp-palette result))))))
(ert-deftest tp-test-deeply-nested-layer-resolution ()
"Test that deeply nested layers (3 levels) are fully resolved."
(tp-test-with-temp-buffer
;; Define 3 levels of nesting
(define-tp tp-test-level1 (val)
`(face (:foreground ,val)))
(define-tp tp-test-level2 (val)
`(tp-test-level1 ,val help-echo "level2"))
(define-tp tp-test-level3 (val)
`(tp-test-level2 ,val display "level3"))
;; Use the most deeply nested layer
(let ((result (tp-set "test" 'tp-test-level3 "blue")))
;; All properties should be resolved
(should (equal (plist-get (get-text-property 0 'face result) :foreground) "blue"))
(should (equal (get-text-property 0 'help-echo result) "level2"))
(should (equal (get-text-property 0 'display result) "level3"))
;; None of the custom layer names should be present
(should (null (get-text-property 0 'tp-test-level1 result)))
(should (null (get-text-property 0 'tp-test-level2 result))))))
(provide 'tp-ert-tests)
;;; tp-ert-tests.el ends here

31
tp.el
View File

@ -2634,7 +2634,8 @@ Also includes tp-name automatically if the layer has reactive dependencies regis
Handles two storage formats:
1. Old format (from tp--set-layer-props): (LAYER-NAME . PLIST) - flat plist
2. Unified format (from define-tp): (LAYER-NAME ARGLIST BODY-FORM)
For parameterized layers (ARGLIST non-nil), returns nil - use `tp-layer-props-with-arg'."
For parameterized layers (ARGLIST non-nil), returns nil - use `tp-layer-props-with-arg'.
Recursively expands any nested layer names in the returned plist."
(when-let ((entry (cdr (assoc layer-name tp-layer-alist))))
;; Auto-include tp-name for layers with reactive deps
(let ((needs-tp-name (or include-tp-name
@ -2654,14 +2655,21 @@ For parameterized layers (ARGLIST non-nil), returns nil - use `tp-layer-props-wi
;; Non-parameterized - evaluate body and return props
(let ((plist (eval body)))
(when plist
;; Recursively expand nested layer names
(when (tp--plist-has-layer-key-p plist)
(setq plist (tp--expand-layer-in-plist plist)))
(if needs-tp-name
(append plist (list 'tp-name layer-name))
plist))))))
;; Old format: entry is just a flat plist
(t
(if needs-tp-name
(append entry (list 'tp-name layer-name))
entry))))))
(let ((plist entry))
;; Recursively expand nested layer names
(when (tp--plist-has-layer-key-p plist)
(setq plist (tp--expand-layer-in-plist plist)))
(if needs-tp-name
(append plist (list 'tp-name layer-name))
plist)))))))
(defun tp-layer-parameterized-p (layer-name)
"Return non-nil if LAYER-NAME is a parameterized layer.
@ -2678,7 +2686,8 @@ where ARGLIST is a non-nil list of argument symbols."
(defun tp-layer-props-with-arg (layer-name arg &optional include-tp-name)
"Return properties for parameterized layer LAYER-NAME with ARG.
Evaluates the body form with the argument bound to the parameter.
If INCLUDE-TP-NAME is non-nil, appends 'tp-name property to identify the layer."
If INCLUDE-TP-NAME is non-nil, appends 'tp-name property to identify the layer.
Recursively expands any nested layer names in the returned plist."
(when-let ((entry (cdr (assoc layer-name tp-layer-alist))))
;; entry is (ARGLIST BODY-FORM)
(let ((arglist (car entry))
@ -2688,6 +2697,9 @@ If INCLUDE-TP-NAME is non-nil, appends 'tp-name property to identify the layer."
;; Evaluate the body with the argument bound
(plist (eval `(let ((,arg-sym ',arg)) ,body))))
(when plist
;; Recursively expand nested layer names
(when (tp--plist-has-layer-key-p plist)
(setq plist (tp--expand-layer-in-plist plist)))
(if include-tp-name
(append plist (list 'tp-name layer-name))
plist)))))))
@ -2706,10 +2718,16 @@ If INCLUDE-TP-NAME is non-nil, each layer's props will include tp-name."
(or (assoc sym tp-layer-alist)
(assoc sym tp-layer-groups))))
(defun tp--plist-has-layer-key-p (plist)
"Return non-nil if PLIST contains any layer names as keys."
(cl-loop for (key _val) on plist by #'cddr
thereis (tp--is-layer-name-p key)))
(defun tp--expand-layer-in-plist (props)
"Expand any layer names found in PROPS plist.
Scans through PROPS treating it as a plist (key value pairs).
When a key is a layer/group name, expands it with its properties.
Recursively expands until no more layer names are found in the result.
Does NOT add tp-name - this is for direct property setting (tp-set/add/reset).
Returns the expanded plist."
(let ((result nil)
@ -2736,6 +2754,9 @@ Returns the expanded plist."
;; Reverse so first layer's properties are applied last (take precedence)
(apply #'append (reverse layer-props-list)))))))
(when layer-props
;; Recursively expand if the layer props contain more layer names
(when (tp--plist-has-layer-key-p layer-props)
(setq layer-props (tp--expand-layer-in-plist layer-props)))
(setq result (append result layer-props)))))
;; Regular property - keep as-is
(t