Fix duplicate function name and test corrections

- Rename second tp--remove-nested-keys to tp--remove-nested-sub-keys
- Fix logic in tp--remove-props-from-string to properly handle nil face subtraction result
- Fix tests to properly use return values from tp-set and tp-remove
- Add face-was-modified tracking variable

Co-authored-by: Kinneyzhang <38454496+Kinneyzhang@users.noreply.github.com>
This commit is contained in:
copilot-swe-agent[bot] 2026-01-06 18:02:35 +00:00
parent b062eb30fd
commit 18b85e446a
2 changed files with 35 additions and 38 deletions

View File

@ -1435,34 +1435,34 @@ Later values should override earlier values for the same sub-property."
(ert-deftest tp-test-remove-entire-string-single-prop ()
"Test tp-remove removes single property from entire string."
(let ((str (tp-set "Hello" 'face 'bold 'help-echo "test")))
(tp-remove str 'face)
(should (null (get-text-property 0 'face str)))
(should (equal (get-text-property 0 'help-echo str) "test"))))
(let* ((str (tp-set "Hello" 'face 'bold 'help-echo "test"))
(result (tp-remove str 'face)))
(should (null (get-text-property 0 'face result)))
(should (equal (get-text-property 0 'help-echo result) "test"))))
(ert-deftest tp-test-remove-entire-string-multiple-props ()
"Test tp-remove removes multiple properties from entire string."
(let ((str (tp-set "Hello" 'face 'bold 'help-echo "test" 'mouse-face 'highlight)))
(tp-remove str 'face 'help-echo)
(should (null (get-text-property 0 'face str)))
(should (null (get-text-property 0 'help-echo str)))
(should (eq (get-text-property 0 'mouse-face str) 'highlight))))
(let* ((str (tp-set "Hello" 'face 'bold 'help-echo "test" 'mouse-face 'highlight))
(result (tp-remove str 'face 'help-echo)))
(should (null (get-text-property 0 'face result)))
(should (null (get-text-property 0 'help-echo result)))
(should (eq (get-text-property 0 'mouse-face result) 'highlight))))
(ert-deftest tp-test-remove-entire-string-sub-prop ()
"Test tp-remove removes sub-property from entire string."
(let ((str (copy-sequence "Hello")))
(put-text-property 0 5 'face '(:foreground "red" :underline t) str)
(tp-remove str 'face :underline)
(let ((face (get-text-property 0 'face str)))
(let* ((str (copy-sequence "Hello"))
(_ (put-text-property 0 5 'face '(:foreground "red" :underline t) str))
(result (tp-remove str 'face :underline)))
(let ((face (get-text-property 0 'face result)))
(should (equal (plist-get face :foreground) "red"))
(should (null (plist-get face :underline))))))
(ert-deftest tp-test-remove-entire-string-nested-sub-prop ()
"Test tp-remove removes nested sub-properties from entire string."
(let ((str (copy-sequence "Hello")))
(put-text-property 0 5 'face '(:foreground "red" :underline (:style wave :color "blue")) str)
(tp-remove str 'face :underline '(:style))
(let* ((face (get-text-property 0 'face str))
(let* ((str (copy-sequence "Hello"))
(_ (put-text-property 0 5 'face '(:foreground "red" :underline (:style wave :color "blue")) str))
(result (tp-remove str 'face :underline '(:style))))
(let* ((face (get-text-property 0 'face result))
(underline (plist-get face :underline)))
(should (equal (plist-get face :foreground) "red"))
(should (equal (plist-get underline :color) "blue"))
@ -1472,11 +1472,11 @@ Later values should override earlier values for the same sub-property."
"Test tp-remove removes a single nested key from a sub-property.
This tests the fix for the bug where (tp-remove str 'face :underline :position)
was removing the entire :underline instead of just :position."
(let ((str (copy-sequence "happy hacking emacs")))
(tp-set str 'face '(:foreground "red" :underline (:position t :color "green"))
'line-prefix ">> " 'other "other")
(tp-remove str 'face :underline :position)
(let* ((face (get-text-property 0 'face str))
(let* ((str (tp-set "happy hacking emacs"
'face '(:foreground "red" :underline (:position t :color "green"))
'line-prefix ">> " 'other "other"))
(result (tp-remove str 'face :underline :position)))
(let* ((face (get-text-property 0 'face result))
(underline (plist-get face :underline)))
;; :foreground should be preserved
(should (equal (plist-get face :foreground) "red"))
@ -1485,8 +1485,8 @@ was removing the entire :underline instead of just :position."
(should (equal (plist-get underline :color) "green"))
(should (null (plist-get underline :position)))
;; Other properties should be preserved
(should (equal (get-text-property 0 'line-prefix str) ">> "))
(should (equal (get-text-property 0 'other str) "other")))))
(should (equal (get-text-property 0 'line-prefix result) ">> "))
(should (equal (get-text-property 0 'other result) "other")))))
;;; ============================================================
;;; New API Tests - Issue 3 & 4: tp-get for strings and new API

25
tp.el
View File

@ -2099,6 +2099,8 @@ Returns a new string (original is not modified)."
(existing-props (text-properties-at start str))
;; Collect face contributions from layers to subtract
(face-to-subtract nil)
;; Track if face was modified by layer subtraction
(face-was-modified nil)
;; Collect all properties to remove entirely (non-face or non-layer)
(props-to-remove-entirely nil))
;; Process each property to remove
@ -2107,15 +2109,13 @@ Returns a new string (original is not modified)."
;; Layer name - get its face contribution and add to subtract list
(let* ((layer-prop-value (plist-get existing-props prop))
(layer-face (tp--get-layer-face-contribution prop layer-prop-value)))
;; Add layer's face to the subtraction list
;; Subtract layer's face from the current face
(when layer-face
(setq face-to-subtract
(tp--subtract-face-from-face-value
(or face-to-subtract (plist-get existing-props 'face))
layer-face))
;; If face-to-subtract is not yet set, initialize it
(unless face-to-subtract
(setq face-to-subtract (plist-get existing-props 'face))))
(let ((current-face (or face-to-subtract (plist-get existing-props 'face))))
(setq face-to-subtract
(tp--subtract-face-from-face-value current-face layer-face))
;; Mark that we processed the face (even if result is nil)
(setq face-was-modified t)))
;; Add the layer property itself to remove list
(push prop props-to-remove-entirely)
;; Also add tp-name if it matches
@ -2124,10 +2124,7 @@ Returns a new string (original is not modified)."
;; Non-layer property - remove entirely
(push prop props-to-remove-entirely)))
;; Build final properties
(let* ((face-was-modified (and face-to-subtract
(not (equal face-to-subtract
(plist-get existing-props 'face)))))
(final-props
(let* ((final-props
(let ((result nil))
(cl-loop for (key val) on existing-props by #'cddr
do (cond
@ -2202,7 +2199,7 @@ Returns a new string (original is not modified)."
(existing-props (text-properties-at start str))
(prop-value (plist-get existing-props property))
(new-value (when (and prop-value (listp prop-value))
(tp--remove-nested-keys prop-value sub-key nested-keys)))
(tp--remove-nested-sub-keys prop-value sub-key nested-keys)))
(final-props (let ((result nil))
(cl-loop for (key val) on existing-props by #'cddr
do (setq result (plist-put result key
@ -2222,7 +2219,7 @@ Returns a new string (original is not modified)."
(tp--remove-props-from-string str start end (list property))))))
(t str)))
(defun tp--remove-nested-keys (plist sub-key nested-keys)
(defun tp--remove-nested-sub-keys (plist sub-key nested-keys)
"Remove NESTED-KEYS from the SUB-KEY value within PLIST.
Returns a new plist (does not modify the original)."
(let* ((sub-value (plist-get plist sub-key))