Address code review feedback

- Fix tp--equal-including-string-properties to only use
  equal-including-properties when BOTH args are strings
- Improve test to use (point-min) and (point-max) instead of
  hardcoded positions

Co-authored-by: Kinneyzhang <38454496+Kinneyzhang@users.noreply.github.com>
This commit is contained in:
copilot-swe-agent[bot] 2026-01-05 19:18:50 +00:00
parent 45268b6e5a
commit 39bbd6b431
2 changed files with 9 additions and 9 deletions

View File

@ -3049,18 +3049,18 @@ text content but different properties, the properties should be updated."
(progn
(define-tp test-same-text-layer ()
:props '(face (:foreground "green") tp-text $tp-test-same-text))
;; Apply layer to text
(insert "placeholder text here")
(tp-set 1 17 'test-same-text-layer)
;; Apply layer to text - insert placeholder and apply layer to entire buffer
(insert "placeholder")
(tp-set (point-min) (point-max) 'test-same-text-layer)
;; Initial text should be "emacs" with foreground green
(should (equal (buffer-substring-no-properties 1 6) "emacs"))
(should (equal (plist-get (tp-at 1 'face) :foreground) "green"))
(should (equal (buffer-substring-no-properties (point-min) (point-max)) "emacs"))
(should (equal (plist-get (tp-at (point-min) 'face) :foreground) "green"))
;; Change the reactive variable to same text but different properties
(setq tp-test-same-text (propertize "emacs" 'face 'bold))
;; Text should still be "emacs"
(should (equal (buffer-substring-no-properties 1 6) "emacs"))
(should (equal (buffer-substring-no-properties (point-min) (point-max)) "emacs"))
;; Face should now include bold from the propertized string
(let ((face-val (tp-at 1 'face)))
(let ((face-val (tp-at (point-min) 'face)))
(should (or (eq face-val 'bold)
(and (listp face-val) (memq 'bold face-val))))))
;; Cleanup

4
tp.el
View File

@ -260,10 +260,10 @@ Scans the entire string, not just position 0."
(defun tp--equal-including-string-properties (a b)
"Compare A and B for equality, considering string text properties.
If either A or B is a string, uses `equal-including-properties' to ensure
If both A and B are strings, uses `equal-including-properties' to ensure
text properties are considered in the comparison.
Otherwise, uses standard `equal'."
(if (or (stringp a) (stringp b))
(if (and (stringp a) (stringp b))
(equal-including-properties a b)
(equal a b)))