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

Previous Next

Full log

Message #28 received at 18175 <at> debbugs.gnu.org (full text, mbox):

From: Ivan Shmakov <ivan <at> siamics.net>
To: 18175 <at> debbugs.gnu.org
Subject: Re: bug#18175: files.el: use mapc in (mapcar 'switch-to-buffer ...)
Date: Thu, 07 Aug 2014 19:15:34 +0000

[Message part 1 (text/plain, inline)]

>>>>> Stefan Monnier <monnier <at> 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

[Message part 2 (text/x-diff, inline)]

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

Previous Next

GNU bug tracking system
Copyright (C) 1999 Darren O. Benham, 1997,2003 nCipher Corporation Ltd, 1994-97 Ian Jackson.