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

[Ivan Shmakov <ivan@siamics.net>]

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

>>>>> Stefan Monnier <monnier@iro.umontreal.ca> 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.

 > Thanks, I think it's indeed a valid/correct optimization, but I
 > really dislike relying on mapc's return value (it really should not
 > return any value at all).

	I tend to disagree with that last part, – it seems like a common
	idiom for a function (or, generally, – a /form/; setq does that,
	for one thing) that’s used “solely” for its side-effects to
	return its “primary” argument, thus allowing for easy
	“chaining”, like:

   (foo (bar 42 (baz data)))

	Instead of:

   (progn
     (foo    data)
     (bar 42 data)
     (baz    data))

	Or something even more crude, like:

   (mapc (lambda (fn)
           (if (consp fn)
               (apply (car fn) `(,@(cdr fn) ,data))
             (funcall fn data)))
         '(foo (bar 42) baz))

	Surely, ‘mapc’ fits that pattern pretty well; cf.:

   (foo (bar  42   (baz lst)))
   (foo (mapc 'qux (baz lst)))

 > In this case, the optimization doesn't seem worth the inconvenient of
 > having a very unusual code (relying on mapc's return value), since
 > those few cons cells we save are drowned in the noise of all the code
 > run by switch-to-buffer.

	Yes.  However, I believe that the last two hunks of the one
	another variant of the diff (MIMEd) actually make the intent to
	return the reverse of the list returned by find-file-noselect
	/clearer,/ – although at the expense of adding one extra LoC in
	each case.

-- 
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: 1345 bytes --]

--- 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)
@@ -1450,8 +1450,9 @@ expand wildcards (if any) and visit multiple files."
     (if (listp value)
 	(progn
 	  (setq value (nreverse value))
-	  (cons (switch-to-buffer-other-window (car value))
-		(mapcar 'switch-to-buffer (cdr value))))
+	  (switch-to-buffer-other-window (car value))
+	  (mapc 'switch-to-buffer (cdr value))
+	  value)
       (switch-to-buffer-other-window value))))
 
 (defun find-file-other-frame (filename &optional wildcards)
@@ -1473,8 +1474,9 @@ expand wildcards (if any) and visit multiple files."
     (if (listp value)
 	(progn
 	  (setq value (nreverse value))
-	  (cons (switch-to-buffer-other-frame (car value))
-		(mapcar 'switch-to-buffer (cdr value))))
+	  (switch-to-buffer-other-frame (car value))
+	  (mapc 'switch-to-buffer (cdr value))
+	  value)
       (switch-to-buffer-other-frame value))))
 
 (defun find-file-existing (filename)