unofficial mirror of bug-gnu-emacs@gnu.org 
 help / color / mirror / code / Atom feed
* bug#63509: [PATCH] Make copy-tree work with records
@ 2023-05-15  3:57 Joseph Turner via Bug reports for GNU Emacs, the Swiss army knife of text editors
  2023-05-15 11:26 ` Eli Zaretskii
  0 siblings, 1 reply; 7+ messages in thread
From: Joseph Turner via Bug reports for GNU Emacs, the Swiss army knife of text editors @ 2023-05-15  3:57 UTC (permalink / raw)
  To: 63509

[-- Attachment #1: Type: text/plain, Size: 367 bytes --]

Hello,

copy-tree does not currently work with records:

(cl-defstruct foo bar)
(let* ((rec (make-foo :bar "hello"))
       (copy (copy-tree rec t)))
  (setf (foo-bar copy) "goodbye")
  (foo-bar rec))

Expected "hello"; actual "goodbye".

Attached patch fixes this behavior.

Please tell me if I misunderstand the intended behavior of copy-tree.

Thank you,

Joseph


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: 0001-Make-copy-tree-work-with-records.patch --]
[-- Type: text/x-diff, Size: 1440 bytes --]

From a20e119a0f60f85039726bf6ebcbe441060f9ef2 Mon Sep 17 00:00:00 2001
From: Joseph Turner <joseph@breatheoutbreathe.in>
Date: Sun, 14 May 2023 21:02:15 -0700
Subject: [PATCH] Make copy-tree work with records

---
 lisp/subr.el | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/lisp/subr.el b/lisp/subr.el
index 03d3324f3d8..9573b19ae7e 100644
--- a/lisp/subr.el
+++ b/lisp/subr.el
@@ -828,7 +828,7 @@ of course, also replace TO with a slightly larger value
   "Make a copy of TREE.
 If TREE is a cons cell, this recursively copies both its car and its cdr.
 Contrast to `copy-sequence', which copies only along the cdrs.  With second
-argument VECP, this copies vectors as well as conses."
+argument VECP, this copies vectors and records as well as conses."
   (declare (side-effect-free error-free))
   (if (consp tree)
       (let (result)
@@ -839,8 +839,8 @@ argument VECP, this copies vectors as well as conses."
 	    (push newcar result))
 	  (setq tree (cdr tree)))
 	(nconc (nreverse result)
-               (if (and vecp (vectorp tree)) (copy-tree tree vecp) tree)))
-    (if (and vecp (vectorp tree))
+               (if (and vecp (or (vectorp tree) (recordp tree))) (copy-tree tree vecp) tree)))
+    (if (and vecp (or (vectorp tree) (recordp tree)))
 	(let ((i (length (setq tree (copy-sequence tree)))))
 	  (while (>= (setq i (1- i)) 0)
 	    (aset tree i (copy-tree (aref tree i) vecp)))
-- 
2.40.1


^ permalink raw reply related	[flat|nested] 7+ messages in thread

* bug#63509: [PATCH] Make copy-tree work with records
  2023-05-15  3:57 bug#63509: [PATCH] Make copy-tree work with records Joseph Turner via Bug reports for GNU Emacs, the Swiss army knife of text editors
@ 2023-05-15 11:26 ` Eli Zaretskii
  2023-05-15 14:42   ` Stefan Monnier via Bug reports for GNU Emacs, the Swiss army knife of text editors
  2023-05-15 17:59   ` Joseph Turner via Bug reports for GNU Emacs, the Swiss army knife of text editors
  0 siblings, 2 replies; 7+ messages in thread
From: Eli Zaretskii @ 2023-05-15 11:26 UTC (permalink / raw)
  To: Joseph Turner, Stefan Monnier; +Cc: 63509

> Date: Sun, 14 May 2023 20:57:17 -0700
> From:  Joseph Turner via "Bug reports for GNU Emacs,
>  the Swiss army knife of text editors" <bug-gnu-emacs@gnu.org>
> 
> copy-tree does not currently work with records:

It isn't documented to work with anything but cons cells or vectors.

> Attached patch fixes this behavior.

Thanks, but such a change must come with:

 - NEWS entry
 - update for the ELisp manual
 - preferably added tests to regression-test this feature

I also think we should rename the second argument, as VECP no longer
fits.

> Please tell me if I misunderstand the intended behavior of copy-tree.

Adding Stefan in case he has comments.





^ permalink raw reply	[flat|nested] 7+ messages in thread

* bug#63509: [PATCH] Make copy-tree work with records
  2023-05-15 11:26 ` Eli Zaretskii
@ 2023-05-15 14:42   ` Stefan Monnier via Bug reports for GNU Emacs, the Swiss army knife of text editors
  2023-05-15 17:59   ` Joseph Turner via Bug reports for GNU Emacs, the Swiss army knife of text editors
  1 sibling, 0 replies; 7+ messages in thread
From: Stefan Monnier via Bug reports for GNU Emacs, the Swiss army knife of text editors @ 2023-05-15 14:42 UTC (permalink / raw)
  To: Eli Zaretskii; +Cc: 63509, Joseph Turner

>> Please tell me if I misunderstand the intended behavior of copy-tree.
> Adding Stefan in case he has comments.

No specific comment, no.  I don't hold `copy-tree` in my heart (it
rarely copies exactly what you need, similar problem as for
`flatten-tree`), so as general rule I'd recommend against its use, but
it's occasionally handy as a quick&dirty "deep copy" function, and
extending it to records seem like an obvious fix (it used to do that
back when `cl-defstruct` used vectors).


        Stefan






^ permalink raw reply	[flat|nested] 7+ messages in thread

* bug#63509: [PATCH] Make copy-tree work with records
  2023-05-15 11:26 ` Eli Zaretskii
  2023-05-15 14:42   ` Stefan Monnier via Bug reports for GNU Emacs, the Swiss army knife of text editors
@ 2023-05-15 17:59   ` Joseph Turner via Bug reports for GNU Emacs, the Swiss army knife of text editors
  2023-05-18 10:53     ` Eli Zaretskii
  1 sibling, 1 reply; 7+ messages in thread
From: Joseph Turner via Bug reports for GNU Emacs, the Swiss army knife of text editors @ 2023-05-15 17:59 UTC (permalink / raw)
  To: Eli Zaretskii; +Cc: 63509, Stefan Monnier

[-- Attachment #1: Type: text/plain, Size: 476 bytes --]


Eli Zaretskii <eliz@gnu.org> writes:

>  - NEWS entry

I updated the 29 entry. Should I move it to 30?

>  - update for the ELisp manual

Changes made in patch:

- updated the entry in lists.texi
- added a new entry in records.texi
- added entry in the vector group in shortdoc.el

>  - preferably added tests to regression-test this feature

Done.

> I also think we should rename the second argument, as VECP no longer
> fits.

How about VECTOR-LIKE-P? See patch.

Joseph


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: 0001-Make-copy-tree-work-with-records.patch --]
[-- Type: text/x-diff, Size: 6502 bytes --]

From 14383e9fe8a107f597d06ad486a441b761cc6ade Mon Sep 17 00:00:00 2001
From: Joseph Turner <joseph@breatheoutbreathe.in>
Date: Sun, 14 May 2023 21:02:15 -0700
Subject: [PATCH] Make copy-tree work with records

---
 doc/lispref/lists.texi      |  9 +++++----
 doc/lispref/records.texi    | 12 ++++++++++++
 etc/NEWS.29                 |  5 +++++
 lisp/emacs-lisp/shortdoc.el |  2 ++
 lisp/subr.el                | 14 +++++++-------
 test/lisp/subr-tests.el     | 31 +++++++++++++++++++++++++++++++
 6 files changed, 62 insertions(+), 11 deletions(-)

diff --git a/doc/lispref/lists.texi b/doc/lispref/lists.texi
index 22a5f7f1239..16ed0358974 100644
--- a/doc/lispref/lists.texi
+++ b/doc/lispref/lists.texi
@@ -696,16 +696,17 @@ not a list, the sequence's elements do not become elements of the
 resulting list.  Instead, the sequence becomes the final @sc{cdr}, like
 any other non-list final argument.
 
-@defun copy-tree tree &optional vecp
+@defun copy-tree tree &optional vector-like-p
 This function returns a copy of the tree @var{tree}.  If @var{tree} is a
 cons cell, this makes a new cons cell with the same @sc{car} and
 @sc{cdr}, then recursively copies the @sc{car} and @sc{cdr} in the
 same way.
 
 Normally, when @var{tree} is anything other than a cons cell,
-@code{copy-tree} simply returns @var{tree}.  However, if @var{vecp} is
-non-@code{nil}, it copies vectors too (and operates recursively on
-their elements).  This function cannot cope with circular lists.
+@code{copy-tree} simply returns @var{tree}.  However, if
+@var{vector-like-p} is non-@code{nil}, it copies vectors and records
+too (and operates recursively on their elements).  This function
+cannot cope with circular lists.
 @end defun
 
 @defun flatten-tree tree
diff --git a/doc/lispref/records.texi b/doc/lispref/records.texi
index 26c6f30a6b5..0f44198a6b0 100644
--- a/doc/lispref/records.texi
+++ b/doc/lispref/records.texi
@@ -81,6 +81,18 @@ This function returns a new record with type @var{type} and
 @end example
 @end defun
 
+@defun copy-tree tree &optional vector-like-p
+This function copies a record when @var{vector-like-p} is
+non-@code{nil}.
+
+@example
+@group
+(copy-tree (record 'foo "a"))
+     @result{} #s(foo "a")
+@end group
+@end example
+@end defun
+
 @node Backward Compatibility
 @section Backward Compatibility
 
diff --git a/etc/NEWS.29 b/etc/NEWS.29
index fa428d9c790..ae9a89203bf 100644
--- a/etc/NEWS.29
+++ b/etc/NEWS.29
@@ -4897,6 +4897,11 @@ Instead, Emacs uses the already-existing 'make-directory' handlers.
 This can let a caller know whether it created DIR.  Formerly,
 'make-directory's return value was unspecified.
 
++++
+** 'copy-tree' now correctly copies records when its optional second
+argument is non-nil.  The second argument has been renamed from VECP
+to VECTOR-LIKE-P since it now works with both vectors and records.
+
 \f
 * Changes in Emacs 29.1 on Non-Free Operating Systems
 
diff --git a/lisp/emacs-lisp/shortdoc.el b/lisp/emacs-lisp/shortdoc.el
index 9a6f5dd12ce..6580e0e4e0c 100644
--- a/lisp/emacs-lisp/shortdoc.el
+++ b/lisp/emacs-lisp/shortdoc.el
@@ -833,6 +833,8 @@ A FUNC form can have any number of `:no-eval' (or `:no-value'),
   (seq-subseq
    :eval (seq-subseq [1 2 3 4 5] 1 3)
    :eval (seq-subseq [1 2 3 4 5] 1))
+  (copy-tree
+   :eval (copy-tree [1 2 3 4]))
   "Mapping Over Vectors"
   (mapcar
    :eval (mapcar #'identity [1 2 3]))
diff --git a/lisp/subr.el b/lisp/subr.el
index 03d3324f3d8..83735933963 100644
--- a/lisp/subr.el
+++ b/lisp/subr.el
@@ -824,26 +824,26 @@ of course, also replace TO with a slightly larger value
                 next (+ from (* n inc)))))
       (nreverse seq))))
 
-(defun copy-tree (tree &optional vecp)
+(defun copy-tree (tree &optional vector-like-p)
   "Make a copy of TREE.
 If TREE is a cons cell, this recursively copies both its car and its cdr.
 Contrast to `copy-sequence', which copies only along the cdrs.  With second
-argument VECP, this copies vectors as well as conses."
+argument VECTOR-LIKE-P, this copies vectors and records as well as conses."
   (declare (side-effect-free error-free))
   (if (consp tree)
       (let (result)
 	(while (consp tree)
 	  (let ((newcar (car tree)))
-	    (if (or (consp (car tree)) (and vecp (vectorp (car tree))))
-		(setq newcar (copy-tree (car tree) vecp)))
+	    (if (or (consp (car tree)) (and vector-like-p (or (vectorp (car tree)) (recordp (car tree)))))
+		(setq newcar (copy-tree (car tree) vector-like-p)))
 	    (push newcar result))
 	  (setq tree (cdr tree)))
 	(nconc (nreverse result)
-               (if (and vecp (vectorp tree)) (copy-tree tree vecp) tree)))
-    (if (and vecp (vectorp tree))
+               (if (and vector-like-p (or (vectorp tree) (recordp tree))) (copy-tree tree vector-like-p) tree)))
+    (if (and vector-like-p (or (vectorp tree) (recordp tree)))
 	(let ((i (length (setq tree (copy-sequence tree)))))
 	  (while (>= (setq i (1- i)) 0)
-	    (aset tree i (copy-tree (aref tree i) vecp)))
+	    (aset tree i (copy-tree (aref tree i) vector-like-p)))
 	  tree)
       tree)))
 
diff --git a/test/lisp/subr-tests.el b/test/lisp/subr-tests.el
index 8f46c2af136..4ebb68556be 100644
--- a/test/lisp/subr-tests.el
+++ b/test/lisp/subr-tests.el
@@ -1206,5 +1206,36 @@ final or penultimate step during initialization."))
     (should (equal a-dedup '("a" "b" "a" "b" "c")))
     (should (eq a a-dedup))))
 
+(ert-deftest subr--copy-tree ()
+  (should (eq (copy-tree nil) nil))
+  (let* ((a (list (list "a") "b" (list "c") "g"))
+         (copy1 (copy-tree a))
+         (copy2 (copy-tree a t)))
+    (should (equal a copy1))
+    (should (equal a copy2))
+    (should-not (eq a copy1))
+    (should-not (eq a copy2)))
+  (let* ((a (list (list "a") "b" (list "c" (record 'foo "d")) (list ["e" "f"]) "g"))
+         (copy1 (copy-tree a))
+         (copy2 (copy-tree a t)))
+    (should (equal a copy1))
+    (should (equal a copy2))
+    (should-not (eq a copy1))
+    (should-not (eq a copy2)))
+  (let* ((a (record 'foo "a" (record 'bar "b")))
+         (copy1 (copy-tree a))
+         (copy2 (copy-tree a t)))
+    (should (equal a copy1))
+    (should (equal a copy2))
+    (should (eq a copy1))
+    (should-not (eq a copy2)))
+  (let* ((a ["a" "b" ["c" ["d"]]])
+         (copy1 (copy-tree a))
+         (copy2 (copy-tree a t)))
+    (should (equal a copy1))
+    (should (equal a copy2))
+    (should (eq a copy1))
+    (should-not (eq a copy2))))
+
 (provide 'subr-tests)
 ;;; subr-tests.el ends here
-- 
2.40.1


^ permalink raw reply related	[flat|nested] 7+ messages in thread

* bug#63509: [PATCH] Make copy-tree work with records
  2023-05-15 17:59   ` Joseph Turner via Bug reports for GNU Emacs, the Swiss army knife of text editors
@ 2023-05-18 10:53     ` Eli Zaretskii
  2023-05-18 19:05       ` Joseph Turner via Bug reports for GNU Emacs, the Swiss army knife of text editors
  0 siblings, 1 reply; 7+ messages in thread
From: Eli Zaretskii @ 2023-05-18 10:53 UTC (permalink / raw)
  To: Joseph Turner; +Cc: 63509, monnier

> From: Joseph Turner <joseph@breatheoutbreathe.in>
> Cc: Stefan Monnier <monnier@iro.umontreal.ca>, 63509@debbugs.gnu.org
> Date: Mon, 15 May 2023 10:59:57 -0700
> 
> Eli Zaretskii <eliz@gnu.org> writes:
> 
> >  - NEWS entry
> 
> I updated the 29 entry. Should I move it to 30?

Yes, this new feature will be installed on the master branch, which
will become Emacs 30.

> --- a/doc/lispref/records.texi
> +++ b/doc/lispref/records.texi
> @@ -81,6 +81,18 @@ This function returns a new record with type @var{type} and
>  @end example
>  @end defun
>  
> +@defun copy-tree tree &optional vector-like-p
> +This function copies a record when @var{vector-like-p} is
> +non-@code{nil}.
> +
> +@example
> +@group
> +(copy-tree (record 'foo "a"))
> +     @result{} #s(foo "a")
> +@end group
> +@end example
> +@end defun

This addition is redundant.  We don't describe the same function in
more than one place.  If there are reasons to mention it in other
places, we just add there a short note with a cross-reference to the
detailed description.

> ++++
> +** 'copy-tree' now correctly copies records when its optional second

The "correctly" part hints that the previous behavior was a bug, which
it wasn't (and we don't mention bugfixes in NEWS anyway).  So I would
rephrase

  'copy-tree' can now copy records as well, when its optional...

> +argument is non-nil.  The second argument has been renamed from VECP
> +to VECTOR-LIKE-P since it now works with both vectors and records.

The last sentence should be removed: we don't mention such minor
details in NEWS, unless the change is an incompatible change.

Last, but not least: please always accompany your changes with
ChageLog-style commit log messages describing the changes.  You can
find more information about this in the file CONTRIBUTE in the Emacs
tree, and you can see many examples by typing "git log" in the
repository.

Thanks.





^ permalink raw reply	[flat|nested] 7+ messages in thread

* bug#63509: [PATCH] Make copy-tree work with records
  2023-05-18 10:53     ` Eli Zaretskii
@ 2023-05-18 19:05       ` Joseph Turner via Bug reports for GNU Emacs, the Swiss army knife of text editors
  2023-05-19  6:07         ` Eli Zaretskii
  0 siblings, 1 reply; 7+ messages in thread
From: Joseph Turner via Bug reports for GNU Emacs, the Swiss army knife of text editors @ 2023-05-18 19:05 UTC (permalink / raw)
  To: Eli Zaretskii; +Cc: 63509, monnier

[-- Attachment #1: Type: text/plain, Size: 1877 bytes --]


Eli Zaretskii <eliz@gnu.org> writes:

> Yes, this new feature will be installed on the master branch, which
> will become Emacs 30.

Moved note from NEWS.29 to NEWS.

>> --- a/doc/lispref/records.texi
>> +++ b/doc/lispref/records.texi
>> @@ -81,6 +81,18 @@ This function returns a new record with type @var{type} and
>>  @end example
>>  @end defun
>>
>> +@defun copy-tree tree &optional vector-like-p
>> +This function copies a record when @var{vector-like-p} is
>> +non-@code{nil}.
>> +
>> +@example
>> +@group
>> +(copy-tree (record 'foo "a"))
>> +     @result{} #s(foo "a")
>> +@end group
>> +@end example
>> +@end defun
>
> This addition is redundant.  We don't describe the same function in
> more than one place.  If there are reasons to mention it in other
> places, we just add there a short note with a cross-reference to the
> detailed description.

Replaced @defun with a short sentence with @pxref.

>> ++++
>> +** 'copy-tree' now correctly copies records when its optional second
>
> The "correctly" part hints that the previous behavior was a bug, which
> it wasn't (and we don't mention bugfixes in NEWS anyway).  So I would
> rephrase
>
>   'copy-tree' can now copy records as well, when its optional...
>
>> +argument is non-nil.  The second argument has been renamed from VECP
>> +to VECTOR-LIKE-P since it now works with both vectors and records.
>
> The last sentence should be removed: we don't mention such minor
> details in NEWS, unless the change is an incompatible change.

Done.

> Last, but not least: please always accompany your changes with
> ChageLog-style commit log messages describing the changes.  You can
> find more information about this in the file CONTRIBUTE in the Emacs
> tree, and you can see many examples by typing "git log" in the
> repository.

Done.

Please let me know if any further changes need to be made!

Best,

Joseph


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: 0001-Make-copy-tree-work-with-records.patch --]
[-- Type: text/x-diff, Size: 6739 bytes --]

From 0ae16ca89e581d3c732607b2daa700d8316a71e3 Mon Sep 17 00:00:00 2001
From: Joseph Turner <joseph@breatheoutbreathe.in>
Date: Sun, 14 May 2023 21:02:15 -0700
Subject: [PATCH] Make copy-tree work with records

* doc/lispref/lists.texi (Building Cons Cells and Lists): Document new
behavior of copy-tree.
* doc/lispref/records.texi (Record Functions): Cross-reference to
lists.texi.
* etc/NEWS: Mention change.  (Bug#63509)
* lisp/emacs-lisp/shortdoc.el: Add copy-tree example to vector group.
* lisp/subr.el (copy-tree): Recurse into records as well as vectors
when optional second argument is non-nil. Rename second argument to
from vecp to vector-like-p.
* test/lisp/subr-tests.el: Test new behavior.
---
 doc/lispref/lists.texi      |  9 +++++----
 doc/lispref/records.texi    |  3 +++
 etc/NEWS                    |  3 +++
 lisp/emacs-lisp/shortdoc.el |  2 ++
 lisp/subr.el                | 14 +++++++-------
 test/lisp/subr-tests.el     | 31 +++++++++++++++++++++++++++++++
 6 files changed, 51 insertions(+), 11 deletions(-)

diff --git a/doc/lispref/lists.texi b/doc/lispref/lists.texi
index 22a5f7f1239..16ed0358974 100644
--- a/doc/lispref/lists.texi
+++ b/doc/lispref/lists.texi
@@ -696,16 +696,17 @@ not a list, the sequence's elements do not become elements of the
 resulting list.  Instead, the sequence becomes the final @sc{cdr}, like
 any other non-list final argument.
 
-@defun copy-tree tree &optional vecp
+@defun copy-tree tree &optional vector-like-p
 This function returns a copy of the tree @var{tree}.  If @var{tree} is a
 cons cell, this makes a new cons cell with the same @sc{car} and
 @sc{cdr}, then recursively copies the @sc{car} and @sc{cdr} in the
 same way.
 
 Normally, when @var{tree} is anything other than a cons cell,
-@code{copy-tree} simply returns @var{tree}.  However, if @var{vecp} is
-non-@code{nil}, it copies vectors too (and operates recursively on
-their elements).  This function cannot cope with circular lists.
+@code{copy-tree} simply returns @var{tree}.  However, if
+@var{vector-like-p} is non-@code{nil}, it copies vectors and records
+too (and operates recursively on their elements).  This function
+cannot cope with circular lists.
 @end defun
 
 @defun flatten-tree tree
diff --git a/doc/lispref/records.texi b/doc/lispref/records.texi
index 26c6f30a6b5..d2c80a27f98 100644
--- a/doc/lispref/records.texi
+++ b/doc/lispref/records.texi
@@ -81,6 +81,9 @@ This function returns a new record with type @var{type} and
 @end example
 @end defun
 
+@code{copy-tree} works with records when its optional second argument
+is non-@code{nil} (@pxref{Building Lists}).
+
 @node Backward Compatibility
 @section Backward Compatibility
 
diff --git a/etc/NEWS b/etc/NEWS
index ce865c9904d..c5063a718b9 100644
--- a/etc/NEWS
+++ b/etc/NEWS
@@ -585,6 +585,9 @@ Since circular alias chains now cannot occur, 'function-alias-p',
 'indirect-function' and 'indirect-variable' will never signal an error.
 Their 'noerror' arguments have no effect and are therefore obsolete.
 
++++
+** 'copy-tree' now copies records when its optional argument is non-nil.
+
 \f
 * Changes in Emacs 30.1 on Non-Free Operating Systems
 
diff --git a/lisp/emacs-lisp/shortdoc.el b/lisp/emacs-lisp/shortdoc.el
index 9a6f5dd12ce..6580e0e4e0c 100644
--- a/lisp/emacs-lisp/shortdoc.el
+++ b/lisp/emacs-lisp/shortdoc.el
@@ -833,6 +833,8 @@ A FUNC form can have any number of `:no-eval' (or `:no-value'),
   (seq-subseq
    :eval (seq-subseq [1 2 3 4 5] 1 3)
    :eval (seq-subseq [1 2 3 4 5] 1))
+  (copy-tree
+   :eval (copy-tree [1 2 3 4]))
   "Mapping Over Vectors"
   (mapcar
    :eval (mapcar #'identity [1 2 3]))
diff --git a/lisp/subr.el b/lisp/subr.el
index 03d3324f3d8..83735933963 100644
--- a/lisp/subr.el
+++ b/lisp/subr.el
@@ -824,26 +824,26 @@ of course, also replace TO with a slightly larger value
                 next (+ from (* n inc)))))
       (nreverse seq))))
 
-(defun copy-tree (tree &optional vecp)
+(defun copy-tree (tree &optional vector-like-p)
   "Make a copy of TREE.
 If TREE is a cons cell, this recursively copies both its car and its cdr.
 Contrast to `copy-sequence', which copies only along the cdrs.  With second
-argument VECP, this copies vectors as well as conses."
+argument VECTOR-LIKE-P, this copies vectors and records as well as conses."
   (declare (side-effect-free error-free))
   (if (consp tree)
       (let (result)
 	(while (consp tree)
 	  (let ((newcar (car tree)))
-	    (if (or (consp (car tree)) (and vecp (vectorp (car tree))))
-		(setq newcar (copy-tree (car tree) vecp)))
+	    (if (or (consp (car tree)) (and vector-like-p (or (vectorp (car tree)) (recordp (car tree)))))
+		(setq newcar (copy-tree (car tree) vector-like-p)))
 	    (push newcar result))
 	  (setq tree (cdr tree)))
 	(nconc (nreverse result)
-               (if (and vecp (vectorp tree)) (copy-tree tree vecp) tree)))
-    (if (and vecp (vectorp tree))
+               (if (and vector-like-p (or (vectorp tree) (recordp tree))) (copy-tree tree vector-like-p) tree)))
+    (if (and vector-like-p (or (vectorp tree) (recordp tree)))
 	(let ((i (length (setq tree (copy-sequence tree)))))
 	  (while (>= (setq i (1- i)) 0)
-	    (aset tree i (copy-tree (aref tree i) vecp)))
+	    (aset tree i (copy-tree (aref tree i) vector-like-p)))
 	  tree)
       tree)))
 
diff --git a/test/lisp/subr-tests.el b/test/lisp/subr-tests.el
index 8f46c2af136..4ebb68556be 100644
--- a/test/lisp/subr-tests.el
+++ b/test/lisp/subr-tests.el
@@ -1206,5 +1206,36 @@ final or penultimate step during initialization."))
     (should (equal a-dedup '("a" "b" "a" "b" "c")))
     (should (eq a a-dedup))))
 
+(ert-deftest subr--copy-tree ()
+  (should (eq (copy-tree nil) nil))
+  (let* ((a (list (list "a") "b" (list "c") "g"))
+         (copy1 (copy-tree a))
+         (copy2 (copy-tree a t)))
+    (should (equal a copy1))
+    (should (equal a copy2))
+    (should-not (eq a copy1))
+    (should-not (eq a copy2)))
+  (let* ((a (list (list "a") "b" (list "c" (record 'foo "d")) (list ["e" "f"]) "g"))
+         (copy1 (copy-tree a))
+         (copy2 (copy-tree a t)))
+    (should (equal a copy1))
+    (should (equal a copy2))
+    (should-not (eq a copy1))
+    (should-not (eq a copy2)))
+  (let* ((a (record 'foo "a" (record 'bar "b")))
+         (copy1 (copy-tree a))
+         (copy2 (copy-tree a t)))
+    (should (equal a copy1))
+    (should (equal a copy2))
+    (should (eq a copy1))
+    (should-not (eq a copy2)))
+  (let* ((a ["a" "b" ["c" ["d"]]])
+         (copy1 (copy-tree a))
+         (copy2 (copy-tree a t)))
+    (should (equal a copy1))
+    (should (equal a copy2))
+    (should (eq a copy1))
+    (should-not (eq a copy2))))
+
 (provide 'subr-tests)
 ;;; subr-tests.el ends here
-- 
2.40.1


^ permalink raw reply related	[flat|nested] 7+ messages in thread

* bug#63509: [PATCH] Make copy-tree work with records
  2023-05-18 19:05       ` Joseph Turner via Bug reports for GNU Emacs, the Swiss army knife of text editors
@ 2023-05-19  6:07         ` Eli Zaretskii
  0 siblings, 0 replies; 7+ messages in thread
From: Eli Zaretskii @ 2023-05-19  6:07 UTC (permalink / raw)
  To: Joseph Turner; +Cc: 63509-done, monnier

> From: Joseph Turner <joseph@breatheoutbreathe.in>
> Cc: monnier@iro.umontreal.ca, 63509@debbugs.gnu.org
> Date: Thu, 18 May 2023 12:05:57 -0700
> 
> Please let me know if any further changes need to be made!

Thanks, installed on master, and closing the bug.





^ permalink raw reply	[flat|nested] 7+ messages in thread

end of thread, other threads:[~2023-05-19  6:07 UTC | newest]

Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2023-05-15  3:57 bug#63509: [PATCH] Make copy-tree work with records Joseph Turner via Bug reports for GNU Emacs, the Swiss army knife of text editors
2023-05-15 11:26 ` Eli Zaretskii
2023-05-15 14:42   ` Stefan Monnier via Bug reports for GNU Emacs, the Swiss army knife of text editors
2023-05-15 17:59   ` Joseph Turner via Bug reports for GNU Emacs, the Swiss army knife of text editors
2023-05-18 10:53     ` Eli Zaretskii
2023-05-18 19:05       ` Joseph Turner via Bug reports for GNU Emacs, the Swiss army knife of text editors
2023-05-19  6:07         ` Eli Zaretskii

Code repositories for project(s) associated with this public inbox

	https://git.savannah.gnu.org/cgit/emacs.git

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).