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

[Ivan Shmakov]

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

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