bug#18175: files.el: use mapc in (mapcar 'switch-to-buffer ...)

[Ivan Shmakov <ivan@siamics.net>]

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

>>>>> Drew Adams <drew.adams@oracle.com> writes:

 >> Given that switch-to-buffer returns its argument, /and/ given that
 >> mapc returns the sequence it’s given, I suggest that the
 >> (mapcar 'switch-to-buffer LIST) forms in lisp/files.el be replaced
 >> with (mapc 'switch-to-buffer LIST), – if only to avoid the
 >> unnecessary consing when the list is effectively copied in the
 >> mapcar case.

 >> The lists mapcar is applied to in such cases are returned from
 >> find-file-noselect, and so, as it seems, are “fresh” ones anyway.

 > Not a good idea, IMHO.

 > It's not just about performance; it's about coding style.

 > By using `mapcar' you are signaling that you are interested in the
 > return values of the argument function (and of course the resulting
 > list of them).

 > By using `mapc' you are signaling that the values returned by the
 > argument function are unimportant (only its side effects are
 > significant).

	How do you signal that the values returned by the argument
	function are unimportant, /and/ that you’re interested in the
	/original/ list instead?

 > If you want to improve the performance, and that is the only change
 > you want to make, then please consider another approach.

	Please consider the patch MIMEd.  FWIW, it avoids one more cons
	in both find-file-other-window and find-file-other-frame.

-- 
FSF associate member #7257  http://boycottsystemd.org/  … 3013 B6A0 230E 334A

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: Type: text/x-diff, Size: 1752 bytes --]

diff --git a/lisp/files.el b/lisp/files.el
index 9272e98..c3a6f0d 100644
--- a/lisp/files.el
+++ b/lisp/files.el
@@ -1428,7 +1428,7 @@ automatically choosing a major mode, use \\[find-file-literally]."
                         (confirm-nonexistent-file-or-buffer)))
   (let ((value (find-file-noselect filename nil nil wildcards)))
     (if (listp value)
-	(mapcar 'switch-to-buffer (nreverse value))
+	(mapc 'switch-to-buffer (nreverse value))
       (switch-to-buffer value))))
 
 (defun find-file-other-window (filename &optional wildcards)
@@ -1448,10 +1448,10 @@ expand wildcards (if any) and visit multiple files."
                         (confirm-nonexistent-file-or-buffer)))
   (let ((value (find-file-noselect filename nil nil wildcards)))
     (if (listp value)
-	(progn
-	  (setq value (nreverse value))
-	  (cons (switch-to-buffer-other-window (car value))
-		(mapcar 'switch-to-buffer (cdr value))))
+	(prog1
+	    (setq value (nreverse value))
+	  (switch-to-buffer-other-window (car value))
+	  (mapc 'switch-to-buffer (cdr value)))
       (switch-to-buffer-other-window value))))
 
 (defun find-file-other-frame (filename &optional wildcards)
@@ -1471,10 +1471,10 @@ expand wildcards (if any) and visit multiple files."
                         (confirm-nonexistent-file-or-buffer)))
   (let ((value (find-file-noselect filename nil nil wildcards)))
     (if (listp value)
-	(progn
-	  (setq value (nreverse value))
-	  (cons (switch-to-buffer-other-frame (car value))
-		(mapcar 'switch-to-buffer (cdr value))))
+	(prog1
+	    (setq value (nreverse value))
+	  (switch-to-buffer-other-frame (car value))
+	  (mapc 'switch-to-buffer (cdr value)))
       (switch-to-buffer-other-frame value))))
 
 (defun find-file-existing (filename)