Refine string modification behavior: region form modifies in-place, entire string form creates copy

Updated the implementation to clearly distinguish:
- (tp-set/add/reset START END PROPS STRING): Modifies original string in-place
- (tp-set/add/reset "string" PROP VAL ...): Creates NEW string using propertize

Also fixed code review issues:
- Removed unused 'offset' variable
- Made tp--remove-nested-keys non-destructive

Updated documentation with detailed behavior table and examples.

Co-authored-by: Kinneyzhang <38454496+Kinneyzhang@users.noreply.github.com>
This commit is contained in:
copilot-swe-agent[bot] 2026-01-06 16:18:38 +00:00
parent 70f60f1a25
commit 98f807993a
3 changed files with 244 additions and 118 deletions

124
README.md
View File

@ -425,26 +425,49 @@ A complete overview of all tp.el functions organized by category:
### Core Property Functions
> **Important: String Modification Behavior**
>
> The core property functions (`tp-set`, `tp-reset`, `tp-add`, `tp-remove`) have different behaviors depending on the calling convention:
>
> | Calling Convention | Underlying Implementation | Modifies Original? |
> |-------------------|---------------------------|-------------------|
> | `(tp-set STRING PROP VAL ...)` | Uses `propertize` internally | **No** - Returns a NEW string |
> | `(tp-set START END PROPS)` | Uses `put-text-property` on buffer | Yes - Modifies current buffer |
> | `(tp-set START END PROPS STRING)` | Uses `put-text-property` on string | **Yes** - Modifies original string |
> | `(tp-set START END PROPS BUFFER)` | Uses `put-text-property` on buffer | Yes - Modifies the buffer |
>
> **Summary:**
> - **Entire string form** `(tp-set "string" ...)`: Creates a **new** propertized string. The original string is not modified. This uses `propertize` internally.
> - **Region form with string object** `(tp-set 0 5 '(...) string)`: **Directly modifies** the original string object using `put-text-property` or `set-text-properties`.
> - **Buffer forms**: Always modify the buffer in-place.
>
> This distinction applies to all core property functions: `tp-set`, `tp-reset`, `tp-add`, and `tp-remove`.
#### `tp-set` - Set Text Properties
Set text properties on a string or buffer region. Replaces only the specified properties, preserving others.
```elisp
;; Current buffer (properties as a list)
;; Current buffer (properties as a list) - modifies buffer in-place
(tp-set START END '(PROPERTY VALUE ...))
(tp-set START END LAYER-NAME)
;; Specific buffer or string
;; Specific buffer or string - modifies OBJECT in-place
(tp-set START END '(PROPERTY VALUE ...) OBJECT)
(tp-set START END LAYER-NAME OBJECT)
;; Entire string (flat properties or layer name)
;; Entire string (flat properties or layer name) - returns NEW string
(tp-set STRING PROPERTY VALUE ...)
(tp-set STRING LAYER-NAME)
```
LAYER-NAME can be a symbol representing a layer defined by `define-tp` or a group defined by `define-tps`.
**Return Values:**
- Buffer forms: Returns `(START . END)` cons cell
- String region form `(tp-set 0 5 '(...) string)`: Returns the modified string (same object)
- Entire string form `(tp-set "string" ...)`: Returns a **new** propertized string
**Examples:**
```elisp
@ -476,14 +499,21 @@ LAYER-NAME can be a symbol representing a layer defined by `define-tp` or a grou
(kill-buffer my-buffer))
;; => (1 . 10)
;; Set properties on a string (0-indexed)
(let ((my-string (tp-set 0 5 '(face italic) "Hello World")))
;; Set properties on a string region (0-indexed) - MODIFIES original string
(let ((my-string (copy-sequence "Hello World")))
(tp-set 0 5 '(face italic) my-string)
my-string)
;; => #("Hello World" 0 5 (face italic))
;; Set properties on entire string
(tp-set "Hello" 'face 'bold 'mouse-face 'highlight)
;; => #("Hello" 0 5 (face bold mouse-face highlight))
;; Set properties on entire string - returns NEW string, original unchanged
(let ((original "Hello"))
(let ((result (tp-set original 'face 'bold)))
(list :original original
:result result
:original-has-props (get-text-property 0 'face original)
:result-has-props (get-text-property 0 'face result))))
;; => (:original "Hello" :result #("Hello" 0 5 (face bold))
;; :original-has-props nil :result-has-props bold)
;; Use a defined layer name on entire string
(define-tp my-style ()
@ -519,13 +549,21 @@ LAYER-NAME can be a symbol representing a layer defined by `define-tp` or a grou
Completely replace ALL text properties with the specified ones.
```elisp
;; Buffer/region forms - modifies in-place
(tp-reset START END '(PROPERTY VALUE ...) &optional OBJECT)
(tp-reset START END LAYER-NAME &optional OBJECT)
;; Entire string form - returns NEW string
(tp-reset STRING PROPERTY VALUE ...)
```
LAYER-NAME can be a symbol representing a layer defined by `define-tp` or a group defined by `define-tps`.
**Return Values:**
- Buffer forms: Returns `(START . END)` cons cell
- String region form: Returns the modified string (same object)
- Entire string form: Returns a **new** propertized string
**Examples:**
```elisp
@ -537,9 +575,12 @@ LAYER-NAME can be a symbol representing a layer defined by `define-tp` or a grou
(tp-at 1))
;; => (face bold) ; help-echo is gone
;; On string
(tp-reset "Hello" 'face 'italic)
;; => #("Hello" 0 5 (face italic))
;; On entire string - returns NEW string, original unchanged
(let ((original "Hello"))
(let ((result (tp-reset original 'face 'italic)))
(list :original-modified (get-text-property 0 'face original)
:result-face (get-text-property 0 'face result))))
;; => (:original-modified nil :result-face italic)
;; Use a defined layer name
(define-tp error-style ()
@ -557,13 +598,21 @@ LAYER-NAME can be a symbol representing a layer defined by `define-tp` or a grou
Add or update properties with deep merge support for nested plists.
```elisp
;; Buffer/region forms - modifies in-place
(tp-add START END '(PROPERTY VALUE ...) &optional OBJECT)
(tp-add START END LAYER-NAME &optional OBJECT)
;; Entire string form - returns NEW string
(tp-add STRING PROPERTY VALUE ...)
```
LAYER-NAME can be a symbol representing a layer defined by `define-tp` or a group defined by `define-tps`.
**Return Values:**
- Buffer forms: Returns `(START . END)` cons cell
- String region form: Returns the modified string (same object)
- Entire string form: Returns a **new** propertized string
**Examples:**
```elisp
@ -583,11 +632,12 @@ LAYER-NAME can be a symbol representing a layer defined by `define-tp` or a grou
(tp-at 1 'face))
;; => (:foreground "red" :background "blue")
;; Face prepending - symbol faces are prepended to face list
(let ((str (tp-set "Hello" 'face 'bold)))
(tp-add str 'face 'shadow)
(tp-at 0 'face str))
;; => (shadow bold)
;; Entire string form - returns NEW string, original unchanged
(let ((original "Hello"))
(let ((result (tp-add original 'face 'bold)))
(list :original-modified (get-text-property 0 'face original)
:result-face (get-text-property 0 'face result))))
;; => (:original-modified nil :result-face bold)
;; Use a defined layer name
(define-tp highlight-style ()
@ -761,21 +811,25 @@ For single-position property queries (previously done with `tp-get`), use `tp-at
Remove a property or nested sub-property from a region or entire string.
```elisp
;; Remove entire property (buffer)
;; Remove entire property (buffer) - modifies in-place
(tp-remove START END PROPERTY &optional OBJECT)
;; Remove sub-property (buffer)
;; Remove sub-property (buffer) - modifies in-place
(tp-remove START END '(PROPERTY SUB-KEY) &optional OBJECT)
;; Remove nested sub-properties (buffer)
;; Remove nested sub-properties (buffer) - modifies in-place
(tp-remove START END '(PROPERTY SUB-KEY (NESTED-KEYS...)) &optional OBJECT)
;; Remove from entire string
;; Remove from entire string - returns NEW string
(tp-remove STRING PROP1 PROP2 ...)
(tp-remove STRING PROPERTY SUB-KEY)
(tp-remove STRING PROPERTY SUB-KEY '(NESTED-KEYS...))
```
**Return Values:**
- Buffer forms: Returns `nil`
- Entire string forms: Returns a **new** string with properties removed
**Examples:**
```elisp
@ -803,24 +857,24 @@ Remove a property or nested sub-property from a region or entire string.
(tp-at 1 '(face :underline)))
;; => (:color "blue") ; :style and :position removed, :color preserved
;; Remove from entire string - multiple properties
(let ((str (tp-set "Hello World" 'face 'bold 'help-echo "tip")))
(tp-remove str 'face 'help-echo)
(tp-at 0 str))
;; => nil
;; Remove from entire string - returns NEW string, original unchanged
(let ((original (propertize "Hello" 'face 'bold 'help-echo "tip")))
(let ((result (tp-remove original 'face)))
(list :original-face (get-text-property 0 'face original)
:result-face (get-text-property 0 'face result))))
;; => (:original-face bold :result-face nil)
;; Remove sub-property from string
(let ((str (copy-sequence "Hello World")))
(tp-set 0 11 '(face (:foreground "red" :underline t)) str)
(tp-remove str 'face :underline)
(tp-at 0 'face str))
;; => (:foreground "red")
;; Remove sub-property from string - returns NEW string
(let ((original (propertize "Hello" 'face '(:foreground "red" :underline t))))
(let ((result (tp-remove original 'face :underline)))
(list :original (get-text-property 0 'face original)
:result (get-text-property 0 'face result))))
;; => (:original (:foreground "red" :underline t) :result (:foreground "red"))
;; Remove nested keys from string
(let ((str (copy-sequence "Hello World")))
(tp-set 0 11 '(face (:underline (:style wave :color "blue"))) str)
(tp-remove str 'face :underline '(:style))
(tp-at 0 '(face :underline) str))
(let ((original (propertize "Hello" 'face '(:underline (:style wave :color "blue")))))
(let ((result (tp-remove original 'face :underline '(:style))))
(get-text-property 0 '(face :underline) result)))
;; => (:color "blue")
```

View File

@ -4095,17 +4095,16 @@ Regression test for: (tp-set \"emacs\" 'face nil) erroring with
;; Strings should not be eq (different objects)
(should (not (eq original result))))))
(ert-deftest tp-test-set-region-does-not-modify-original-string ()
"Test that tp-set with region returns a new string and does not modify the original."
(let ((original "Hello World"))
(ert-deftest tp-test-set-region-modifies-original-string ()
"Test that tp-set with region form DOES modify the original string.
The region form (tp-set START END PROPS STRING) modifies the string in-place."
(let ((original (copy-sequence "Hello World")))
(let ((result (tp-set 0 5 '(face bold) original)))
;; Result should be a new string with properties on the region
(should (stringp result))
;; Result should be the same object as original (modified in-place)
(should (eq result original))
;; Both should have the face property
(should (eq (get-text-property 0 'face result) 'bold))
;; Original should NOT be modified (no properties)
(should (null (get-text-property 0 'face original)))
;; Strings should not be eq (different objects)
(should (not (eq original result))))))
(should (eq (get-text-property 0 'face original) 'bold)))))
(ert-deftest tp-test-match-set-does-not-modify-original-string ()
"Test that tp-match-set returns a new string and does not modify the original."

221
tp.el
View File

@ -1305,49 +1305,82 @@ PROPS can be a plist or a layer/group name symbol.
Preserves existing properties not specified in PROPS.
For tp-text, props override embedded text properties.
For strings, returns a NEW propertized string (original is not modified).
For buffers, returns (START . END) cons."
(pcase-let ((`(,object ,start ,finish ,props)
(tp--parse-args start-or-string end-or-prop props-or-val rest)))
;; Handle tp-text property specially - :override means props override embedded props
(pcase-let ((`(,new-props ,new-finish ,new-object)
(tp--handle-tp-text-property start finish props object t :override)))
(setq props new-props finish new-finish object new-object)
(when (and (stringp object) (plist-member props 'tp-text))
(setq start 0)))
(if (stringp object)
;; For strings: create a new propertized string (non-destructive)
(tp--apply-props-to-string object start finish props nil)
;; For buffers: modify in place (standard behavior)
(let ((has-existing-props (text-properties-at start object)))
(if (and (not has-existing-props)
(= start (or (next-single-property-change start nil object finish) finish)))
(set-text-properties start finish props object)
(cl-loop for (key val) on props by #'cddr
do (put-text-property start finish key val object))))
(cons start finish))))
**String Modification Behavior:**
- Entire string form (tp-set STRING ...): Returns a NEW propertized string
(original is not modified). Uses `propertize' internally.
- Region form with string (tp-set START END PROPS STRING): Modifies the
original string in-place using `put-text-property'.
- Buffer forms: Always modify in-place.
Returns: For buffers, (START . END) cons. For strings, the result string."
;; Determine if this is the "entire string" form (first arg is a string)
(let ((entire-string-form (stringp start-or-string)))
(pcase-let ((`(,object ,start ,finish ,props)
(tp--parse-args start-or-string end-or-prop props-or-val rest)))
;; Handle tp-text property specially - :override means props override embedded props
(pcase-let ((`(,new-props ,new-finish ,new-object)
(tp--handle-tp-text-property start finish props object t :override)))
(setq props new-props finish new-finish object new-object)
(when (and (stringp object) (plist-member props 'tp-text))
(setq start 0)))
(cond
;; Entire string form: create a new propertized string (non-destructive)
((and (stringp object) entire-string-form)
(tp--apply-props-to-string object start finish props nil))
;; Region form with string object: modify in-place
((stringp object)
(let ((has-existing-props (text-properties-at start object)))
(if (and (not has-existing-props)
(= start (or (next-single-property-change start nil object finish) finish)))
(set-text-properties start finish props object)
(cl-loop for (key val) on props by #'cddr
do (put-text-property start finish key val object))))
object)
;; Buffer: modify in place
(t
(let ((has-existing-props (text-properties-at start object)))
(if (and (not has-existing-props)
(= start (or (next-single-property-change start nil object finish) finish)))
(set-text-properties start finish props object)
(cl-loop for (key val) on props by #'cddr
do (put-text-property start finish key val object))))
(cons start finish))))))
(defun tp-reset (start-or-string &optional end-or-prop props-or-val &rest rest)
"Completely replace all text properties with PROPS.
Like `tp-set' but replaces ALL existing properties.
For tp-text, embedded text properties are ignored - only props are used.
For strings, returns a NEW propertized string (original is not modified).
For buffers, returns (START . END) cons."
(pcase-let ((`(,object ,start ,finish ,props)
(tp--parse-args start-or-string end-or-prop props-or-val rest)))
;; Handle tp-text property - :reset means only use props, ignore embedded props
(pcase-let ((`(,new-props ,new-finish ,new-object)
(tp--handle-tp-text-property start finish props object nil :reset)))
(setq props new-props finish new-finish object new-object)
(when (and (stringp object) (plist-member props 'tp-text))
(setq start 0)))
(if (stringp object)
;; For strings: create a new propertized string (non-destructive)
(tp--apply-props-to-string object start finish props :reset)
;; For buffers: modify in place (standard behavior)
(set-text-properties start finish props object)
(cons start finish))))
**String Modification Behavior:**
- Entire string form (tp-reset STRING ...): Returns a NEW propertized string
(original is not modified). Uses `propertize' internally.
- Region form with string (tp-reset START END PROPS STRING): Modifies the
original string in-place using `set-text-properties'.
- Buffer forms: Always modify in-place.
Returns: For buffers, (START . END) cons. For strings, the result string."
;; Determine if this is the "entire string" form (first arg is a string)
(let ((entire-string-form (stringp start-or-string)))
(pcase-let ((`(,object ,start ,finish ,props)
(tp--parse-args start-or-string end-or-prop props-or-val rest)))
;; Handle tp-text property - :reset means only use props, ignore embedded props
(pcase-let ((`(,new-props ,new-finish ,new-object)
(tp--handle-tp-text-property start finish props object nil :reset)))
(setq props new-props finish new-finish object new-object)
(when (and (stringp object) (plist-member props 'tp-text))
(setq start 0)))
(cond
;; Entire string form: create a new propertized string (non-destructive)
((and (stringp object) entire-string-form)
(tp--apply-props-to-string object start finish props :reset))
;; Region form with string object: modify in-place
((stringp object)
(set-text-properties start finish props object)
object)
;; Buffer: modify in place
(t
(set-text-properties start finish props object)
(cons start finish))))))
(defun tp--prepend-face (new-face existing-face)
"Prepend NEW-FACE to EXISTING-FACE for the face property.
@ -1438,41 +1471,69 @@ Unlike `tp-set', deeply merges nested properties.
For `face' property, symbol faces are prepended to existing face list.
For tp-text, embedded text properties are merged with props.
For strings, returns a NEW propertized string (original is not modified).
For buffers, returns (START . END) cons."
(pcase-let ((`(,object ,start ,finish ,props)
(tp--parse-args start-or-string end-or-prop props-or-val rest)))
;; Handle tp-text property - :merge means embedded props are merged with props
(let ((has-tp-text (plist-member props 'tp-text)))
(pcase-let ((`(,new-props ,new-finish ,new-object)
(tp--handle-tp-text-property start finish props object t :merge)))
(setq props new-props finish new-finish object new-object)
(when (and (stringp object) has-tp-text)
(setq start 0))))
(if (stringp object)
;; For strings: create a new propertized string (non-destructive)
**String Modification Behavior:**
- Entire string form (tp-add STRING ...): Returns a NEW propertized string
(original is not modified). Uses `propertize' internally.
- Region form with string (tp-add START END PROPS STRING): Modifies the
original string in-place using `put-text-property'.
- Buffer forms: Always modify in-place.
Returns: For buffers, (START . END) cons. For strings, the result string."
;; Determine if this is the "entire string" form (first arg is a string)
(let ((entire-string-form (stringp start-or-string)))
(pcase-let ((`(,object ,start ,finish ,props)
(tp--parse-args start-or-string end-or-prop props-or-val rest)))
;; Handle tp-text property - :merge means embedded props are merged with props
(let ((has-tp-text (plist-member props 'tp-text)))
(pcase-let ((`(,new-props ,new-finish ,new-object)
(tp--handle-tp-text-property start finish props object t :merge)))
(setq props new-props finish new-finish object new-object)
(when (and (stringp object) has-tp-text)
(setq start 0))))
(cond
;; Entire string form: create a new propertized string (non-destructive)
((and (stringp object) entire-string-form)
(if (plist-member props 'tp-text)
;; For tp-text, properties are already merged
(tp--apply-props-to-string object start finish props :reset)
;; Otherwise use :add mode for deep merging
(tp--apply-props-to-string object start finish props :add))
;; For buffers: modify in place with deep merging
(let ((pos start))
(while (< pos finish)
(let* ((current-props (text-properties-at pos object))
(next-pos (or (next-property-change pos object finish) finish)))
(cl-loop
for (key val) on props by #'cddr
do (let* ((current-val (plist-get current-props key))
(new-val (cond
((eq key 'face) (tp--prepend-face val current-val))
((and (listp val) (keywordp (car-safe val))
(listp current-val) (keywordp (car-safe current-val)))
(tp--deep-merge-plist current-val val))
(t val))))
(put-text-property pos next-pos key new-val object)))
(setq pos next-pos))))
(cons start finish))))
(tp--apply-props-to-string object start finish props :add)))
;; Region form with string object: modify in-place with deep merging
((stringp object)
(let ((pos start))
(while (< pos finish)
(let* ((current-props (text-properties-at pos object))
(next-pos (or (next-property-change pos object finish) finish)))
(cl-loop
for (key val) on props by #'cddr
do (let* ((current-val (plist-get current-props key))
(new-val (cond
((eq key 'face) (tp--prepend-face val current-val))
((and (listp val) (keywordp (car-safe val))
(listp current-val) (keywordp (car-safe current-val)))
(tp--deep-merge-plist current-val val))
(t val))))
(put-text-property pos next-pos key new-val object)))
(setq pos next-pos))))
object)
;; Buffer: modify in place with deep merging
(t
(let ((pos start))
(while (< pos finish)
(let* ((current-props (text-properties-at pos object))
(next-pos (or (next-property-change pos object finish) finish)))
(cl-loop
for (key val) on props by #'cddr
do (let* ((current-val (plist-get current-props key))
(new-val (cond
((eq key 'face) (tp--prepend-face val current-val))
((and (listp val) (keywordp (car-safe val))
(listp current-val) (keywordp (car-safe current-val)))
(tp--deep-merge-plist current-val val))
(t val))))
(put-text-property pos next-pos key new-val object)))
(setq pos next-pos))))
(cons start finish))))))
;;;============================================================================
;;; Layer 2: Core Property Functions - Get/At
@ -1770,8 +1831,14 @@ This function supports multiple calling conventions:
(tp-remove STRING PROPERTY SUB-KEY \\='(NESTED-KEYS...))
(tp-remove \"Hello\" \\='face :underline \\='(:style :position))
For strings, returns a NEW string with properties removed (original is not modified).
For buffers, returns nil."
**String Modification Behavior:**
- Entire string form (tp-remove STRING ...): Returns a NEW string with
properties removed (original is not modified). Uses `propertize' internally.
- Region form with string (tp-remove START END PROP STRING): Modifies the
original string in-place using `remove-text-properties'.
- Buffer forms: Always modify in-place.
Returns: For buffers, nil. For entire string forms, a new string."
(cond
;; First arg is a string - apply to entire string, non-destructively
((stringp start-or-string)
@ -1916,7 +1983,7 @@ Returns a new string (original is not modified)."
(defun tp--remove-nested-keys (plist sub-key nested-keys)
"Remove NESTED-KEYS from the SUB-KEY value within PLIST.
Returns the modified plist."
Returns a new plist (does not modify the original)."
(let* ((sub-value (plist-get plist sub-key))
(keys-to-remove (if (listp nested-keys) nested-keys (list nested-keys)))
(new-sub-value (when (and sub-value (listp sub-value))
@ -1926,7 +1993,14 @@ Returns the modified plist."
do (setq result (plist-put result k v)))
result))))
(if new-sub-value
(plist-put plist sub-key new-sub-value)
;; Build a new plist with the updated sub-value
(let ((result nil))
(cl-loop for (k v) on plist by #'cddr
do (setq result (plist-put result k
(if (eq k sub-key)
new-sub-value
v))))
result)
;; Remove the sub-key entirely if no value left
(let ((result nil))
(cl-loop for (k v) on plist by #'cddr
@ -1955,8 +2029,7 @@ For buffers, modifies in-place and returns list of regions."
;; String object
((stringp object)
(let ((result object)
(pos 0)
(offset 0)) ; Track offset for position changes (though properties shouldn't change length)
(pos 0))
(while (string-match (regexp-quote pattern) result pos)
(let ((beg (match-beginning 0))
(end (match-end 0)))