unofficial mirror of bug-gnu-emacs@gnu.org 
 help / color / mirror / code / Atom feed
* bug#28867: Replace gnus-copy-sequence with copy-tree
@ 2017-10-16 16:58 Eric Abrahamsen
       [not found] ` <handler.28867.B.150817319123733.ack@debbugs.gnu.org>
  0 siblings, 1 reply; 6+ messages in thread
From: Eric Abrahamsen @ 2017-10-16 16:58 UTC (permalink / raw)
  To: 28867

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

As with my previous report, this is part of a push to use core functions
rather than Gnus-specific implementations (so long as the behavior is
the same). `copy-tree' does everything `gnus-copy-sequence' does.


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: 0001-Replace-gnus-copy-sequence-with-copy-tree.patch --]
[-- Type: text/x-diff, Size: 7909 bytes --]

From c4262a8d84d1b9db708edffb05944d5f77f29844 Mon Sep 17 00:00:00 2001
From: Eric Abrahamsen <eric@ericabrahamsen.net>
Date: Sun, 15 Oct 2017 12:53:02 -0700
Subject: [PATCH] Replace gnus-copy-sequence with copy-tree

Use built-in equivalents where possible.

* lisp/gnus/gnus-range.el: Delete gnus-copy-sequence.
  (gnus-remove-from-range)
* lisp/gnus/gnus-agent.el (gnus-agent-catchup,
  gnus-agent-summary-fetch-series, gnus-category-copy)
* lisp/gnus/gnus-cus.el (gnus-group-customize)
* lisp/gnus/gnus-group.el (gnus-group-edit-group-done,
  gnus-group-make-useful-group)
* lisp/gnus/gnus-score.el (gnus-score-adaptive)
* lisp/gnus/gnus-srvr.el (gnus-server-copy-server)
* lisp/gnus/gnus-sum.el (gnus-summary-read-group-1, gnus-update-marks,
  gnus-summary-insert-new-articles): Replace with copy-tree in all
  these locations.
---
 lisp/gnus/gnus-agent.el |  6 +++---
 lisp/gnus/gnus-cus.el   |  2 +-
 lisp/gnus/gnus-group.el |  4 ++--
 lisp/gnus/gnus-range.el | 13 +------------
 lisp/gnus/gnus-score.el |  2 +-
 lisp/gnus/gnus-srvr.el  |  2 +-
 lisp/gnus/gnus-sum.el   |  8 ++++----
 lisp/gnus/gnus.el       |  1 -
 8 files changed, 13 insertions(+), 25 deletions(-)

diff --git a/lisp/gnus/gnus-agent.el b/lisp/gnus/gnus-agent.el
index daf578180f..f2dfe07c4c 100644
--- a/lisp/gnus/gnus-agent.el
+++ b/lisp/gnus/gnus-agent.el
@@ -1108,7 +1108,7 @@ gnus-agent-catchup
                 gnus-newsgroup-cached)
         (setq articles (gnus-sorted-ndifference
 			(gnus-sorted-ndifference
-			 (gnus-copy-sequence articles)
+			 (copy-tree articles)
 			 gnus-newsgroup-downloadable)
 			gnus-newsgroup-cached)))
 
@@ -1123,7 +1123,7 @@ gnus-agent-summary-fetch-series
   (when gnus-newsgroup-processable
     (setq gnus-newsgroup-downloadable
           (let* ((dl gnus-newsgroup-downloadable)
-		 (processable (sort (gnus-copy-sequence gnus-newsgroup-processable) '<))
+		 (processable (sort (copy-tree gnus-newsgroup-processable) '<))
                  (gnus-newsgroup-downloadable processable))
 	    (gnus-agent-summary-fetch-group)
 
@@ -2833,7 +2833,7 @@ gnus-category-copy
   "Copy the current category."
   (interactive (list (gnus-category-name) (intern (read-string "New name: "))))
   (let ((info (assq category gnus-category-alist)))
-    (push (let ((newcat (gnus-copy-sequence info)))
+    (push (let ((newcat (copy-tree info)))
             (setf (gnus-agent-cat-name newcat) to)
             (setf (gnus-agent-cat-groups newcat) nil)
             newcat)
diff --git a/lisp/gnus/gnus-cus.el b/lisp/gnus/gnus-cus.el
index 600b33f226..c22c9c1d5a 100644
--- a/lisp/gnus/gnus-cus.el
+++ b/lisp/gnus/gnus-cus.el
@@ -406,7 +406,7 @@ gnus-group-customize
       ;; every duplicate ends up being displayed.  So, rather than
       ;; display them, remove them from the list.
 
-      (let ((tmp (setq values (gnus-copy-sequence values)))
+      (let ((tmp (setq values (copy-tree values)))
 	    elem)
 	(while (cdr tmp)
 	  (while (setq elem (assq (caar tmp) (cdr tmp)))
diff --git a/lisp/gnus/gnus-group.el b/lisp/gnus/gnus-group.el
index 985efe6272..04f8244b63 100644
--- a/lisp/gnus/gnus-group.el
+++ b/lisp/gnus/gnus-group.el
@@ -2993,7 +2993,7 @@ gnus-group-edit-group-done
     ;; Set the info.
     (if (not (and info new-group))
 	(gnus-group-set-info form (or new-group group) part)
-      (setq info (gnus-copy-sequence info))
+      (setq info (copy-tree info))
       (setcar info new-group)
       (unless (gnus-server-equal method "native")
 	(unless (nthcdr 3 info)
@@ -3016,7 +3016,7 @@ gnus-group-make-useful-group
 	   ;; Don't use `caddr' here since macros within the `interactive'
 	   ;; form won't be expanded.
 	   (car (cddr entry)))))
-  (setq method (gnus-copy-sequence method))
+  (setq method (copy-tree method))
   (let (entry)
     (while (setq entry (memq (assq 'eval method) method))
       (setcar entry (eval (cadar entry)))))
diff --git a/lisp/gnus/gnus-range.el b/lisp/gnus/gnus-range.el
index b30b2e9099..cc094b7862 100644
--- a/lisp/gnus/gnus-range.el
+++ b/lisp/gnus/gnus-range.el
@@ -39,17 +39,6 @@ gnus-last-element
     (setq list (cdr list)))
   (car list))
 
-(defun gnus-copy-sequence (list)
-  "Do a complete, total copy of a list."
-  (let (out)
-    (while (consp list)
-      (if (consp (car list))
-	  (push (gnus-copy-sequence (pop list)) out)
-	(push (pop list) out)))
-    (if list
-	(nconc (nreverse out) list)
-      (nreverse out))))
-
 (defun gnus-set-difference (list1 list2)
   "Return a list of elements of LIST1 that do not appear in LIST2."
   (let ((hash2 (make-hash-table :test 'eq))
@@ -455,7 +444,7 @@ gnus-remove-from-range
   (if (or (null range1) (null range2))
       range1
     (let (out r1 r2 r1_min r1_max r2_min r2_max
-	      (range2 (gnus-copy-sequence range2)))
+	      (range2 (copy-tree range2)))
       (setq range1 (if (listp (cdr range1)) range1 (list range1))
 	    range2 (sort (if (listp (cdr range2)) range2 (list range2))
 			 (lambda (e1 e2)
diff --git a/lisp/gnus/gnus-score.el b/lisp/gnus/gnus-score.el
index 976ac9f7f3..d1f065748c 100644
--- a/lisp/gnus/gnus-score.el
+++ b/lisp/gnus/gnus-score.el
@@ -2318,7 +2318,7 @@ gnus-score-adaptive
     (when (or (not (listp gnus-newsgroup-adaptive))
 	      (memq 'line gnus-newsgroup-adaptive))
       (save-excursion
-	(let* ((malist (gnus-copy-sequence gnus-adaptive-score-alist))
+	(let* ((malist (copy-tree gnus-adaptive-score-alist))
 	       (alist malist)
 	       (date (current-time-string))
 	       (data gnus-newsgroup-data)
diff --git a/lisp/gnus/gnus-srvr.el b/lisp/gnus/gnus-srvr.el
index 82056cf165..67702d6028 100644
--- a/lisp/gnus/gnus-srvr.el
+++ b/lisp/gnus/gnus-srvr.el
@@ -608,7 +608,7 @@ gnus-server-copy-server
     (error "%s already exists" to))
   (unless (gnus-server-to-method from)
     (error "%s: no such server" from))
-  (let ((to-entry (cons from (gnus-copy-sequence
+  (let ((to-entry (cons from (copy-tree
 			      (gnus-server-to-method from)))))
     (setcar to-entry to)
     (setcar (nthcdr 2 to-entry) to)
diff --git a/lisp/gnus/gnus-sum.el b/lisp/gnus/gnus-sum.el
index 48571096cc..763cb31578 100644
--- a/lisp/gnus/gnus-sum.el
+++ b/lisp/gnus/gnus-sum.el
@@ -3992,7 +3992,7 @@ gnus-summary-read-group-1
 	(spam-initialize))
       ;; Save the active value in effect when the group was entered.
       (setq gnus-newsgroup-active
-	    (gnus-copy-sequence
+	    (copy-tree
 	     (gnus-active gnus-newsgroup-name)))
       (setq gnus-newsgroup-highest (cdr gnus-newsgroup-active))
       ;; You can change the summary buffer in some way with this hook.
@@ -6076,12 +6076,12 @@ gnus-update-marks
 		 (del
 		  (gnus-list-range-intersection
 		   gnus-newsgroup-articles
-		   (gnus-remove-from-range (gnus-copy-sequence old) list)))
+		   (gnus-remove-from-range (copy-tree old) list)))
 		 (add
 		  (gnus-list-range-intersection
 		   gnus-newsgroup-articles
 		   (gnus-remove-from-range
-		    (gnus-copy-sequence list) old))))
+		    (copy-tree list) old))))
 	    (when add
 	      (push (list add 'add (list (cdr type))) delta-marks))
 	    (when del
@@ -13002,7 +13002,7 @@ gnus-summary-insert-new-articles
 	i new)
     (unless new-active
       (error "Couldn't fetch new data"))
-    (setq gnus-newsgroup-active (gnus-copy-sequence new-active))
+    (setq gnus-newsgroup-active (copy-tree new-active))
     (setq i (cdr gnus-newsgroup-active)
 	  gnus-newsgroup-highest i)
     (while (> i old-high)
diff --git a/lisp/gnus/gnus.el b/lisp/gnus/gnus.el
index 8c0846be9f..c27c57fb95 100644
--- a/lisp/gnus/gnus.el
+++ b/lisp/gnus/gnus.el
@@ -2902,7 +2902,6 @@ gnus-other-frame-object
       gnus-check-reasonable-setup)
      ("gnus-dup" gnus-dup-suppress-articles gnus-dup-unsuppress-article
       gnus-dup-enter-articles)
-     ("gnus-range" gnus-copy-sequence)
      ("gnus-eform" gnus-edit-form)
      ("gnus-logic" gnus-score-advanced)
      ("gnus-undo" gnus-undo-mode gnus-undo-register)
-- 
2.14.2


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

* bug#28867: Acknowledgement (Replace gnus-copy-sequence with copy-tree)
       [not found] ` <handler.28867.B.150817319123733.ack@debbugs.gnu.org>
@ 2017-10-16 17:02   ` Eric Abrahamsen
  2017-10-19 10:44     ` Noam Postavsky
  0 siblings, 1 reply; 6+ messages in thread
From: Eric Abrahamsen @ 2017-10-16 17:02 UTC (permalink / raw)
  To: 28867

tags 28867 patch





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

* bug#28867: Acknowledgement (Replace gnus-copy-sequence with copy-tree)
  2017-10-16 17:02   ` bug#28867: Acknowledgement (Replace gnus-copy-sequence with copy-tree) Eric Abrahamsen
@ 2017-10-19 10:44     ` Noam Postavsky
  2017-10-19 15:05       ` Eric Abrahamsen
  2017-11-22  1:29       ` Eric Abrahamsen
  0 siblings, 2 replies; 6+ messages in thread
From: Noam Postavsky @ 2017-10-19 10:44 UTC (permalink / raw)
  To: Eric Abrahamsen; +Cc: 28867

tags 28867 + patch
tags 28856 + patch
quit

Both patches look fine, the only possibly problem I can see is if some
other packge relies on the gnus functions you are removing.  Probably
not an issue, since they're so trivially replaced by builtins anyway.

Eric Abrahamsen <eric@ericabrahamsen.net> writes:

> tags 28867 patch

You have to send this to control@debbugs.gnu.org (which I've Bcc'd on
this email).





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

* bug#28867: Acknowledgement (Replace gnus-copy-sequence with copy-tree)
  2017-10-19 10:44     ` Noam Postavsky
@ 2017-10-19 15:05       ` Eric Abrahamsen
  2017-10-19 15:10         ` Eric Abrahamsen
  2017-11-22  1:29       ` Eric Abrahamsen
  1 sibling, 1 reply; 6+ messages in thread
From: Eric Abrahamsen @ 2017-10-19 15:05 UTC (permalink / raw)
  To: 28867

Noam Postavsky <npostavs@users.sourceforge.net> writes:

> tags 28867 + patch
> tags 28856 + patch
> quit
>
> Both patches look fine, the only possibly problem I can see is if some
> other packge relies on the gnus functions you are removing.  Probably
> not an issue, since they're so trivially replaced by builtins anyway.

FWIW, I checked all the packages in Elpa/Melpa that mention Gnus, and
none of them use either of the functions.

> Eric Abrahamsen <eric@ericabrahamsen.net> writes:
>
>> tags 28867 patch
>
> You have to send this to control@debbugs.gnu.org (which I've Bcc'd on
> this email).

Ah, I could tell I hadn't quite got that right, thanks.






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

* bug#28867: Acknowledgement (Replace gnus-copy-sequence with copy-tree)
  2017-10-19 15:05       ` Eric Abrahamsen
@ 2017-10-19 15:10         ` Eric Abrahamsen
  0 siblings, 0 replies; 6+ messages in thread
From: Eric Abrahamsen @ 2017-10-19 15:10 UTC (permalink / raw)
  To: 28867

Eric Abrahamsen <eric@ericabrahamsen.net> writes:

> Noam Postavsky <npostavs@users.sourceforge.net> writes:
>
>> tags 28867 + patch
>> tags 28856 + patch
>> quit
>>
>> Both patches look fine, the only possibly problem I can see is if some
>> other packge relies on the gnus functions you are removing.  Probably
>> not an issue, since they're so trivially replaced by builtins anyway.
>
> FWIW, I checked all the packages in Elpa/Melpa that mention Gnus, and
> none of them use either of the functions.

Oh I lied, gnus-desktop-notify has a single call to `gnus-last-element'.
I can leave them a note.






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

* bug#28867: Acknowledgement (Replace gnus-copy-sequence with copy-tree)
  2017-10-19 10:44     ` Noam Postavsky
  2017-10-19 15:05       ` Eric Abrahamsen
@ 2017-11-22  1:29       ` Eric Abrahamsen
  1 sibling, 0 replies; 6+ messages in thread
From: Eric Abrahamsen @ 2017-11-22  1:29 UTC (permalink / raw)
  To: Noam Postavsky; +Cc: 28867


On 10/19/17 06:44 AM, Noam Postavsky wrote:
> tags 28867 + patch
> tags 28856 + patch
> quit
>
> Both patches look fine, the only possibly problem I can see is if some
> other packge relies on the gnus functions you are removing.  Probably
> not an issue, since they're so trivially replaced by builtins anyway.
>
> Eric Abrahamsen <eric@ericabrahamsen.net> writes:
>
>> tags 28867 patch
>
> You have to send this to control@debbugs.gnu.org (which I've Bcc'd on
> this email).

What I'd like to do with this and also bug #28856 (regarding
gnus-last-element) is to leave the function definitions in place,
obsolete them, and remove all the calls. 

I alerted one third-party package that was using gnus-last-element to
the impending change, and haven't found any others, but it seems like
obsoleting the functions is still safer.

I'll do that tomorrow, unless anyone objects.





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

end of thread, other threads:[~2017-11-22  1:29 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-10-16 16:58 bug#28867: Replace gnus-copy-sequence with copy-tree Eric Abrahamsen
     [not found] ` <handler.28867.B.150817319123733.ack@debbugs.gnu.org>
2017-10-16 17:02   ` bug#28867: Acknowledgement (Replace gnus-copy-sequence with copy-tree) Eric Abrahamsen
2017-10-19 10:44     ` Noam Postavsky
2017-10-19 15:05       ` Eric Abrahamsen
2017-10-19 15:10         ` Eric Abrahamsen
2017-11-22  1:29       ` Eric Abrahamsen

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).