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

Previous Next

Package: emacs;

Reported by: Ivan Shmakov <ivan <at> siamics.net>

Date: Sat, 2 Aug 2014 21:56:02 UTC

Severity: wishlist

Tags: patch

Fixed in version 25.1

Done: Ivan Shmakov <ivan <at> siamics.net>

Bug is archived. No further changes may be made.

Full log


View this message in rfc822 format

From: Ivan Shmakov <ivan <at> siamics.net>
To: 18175 <at> debbugs.gnu.org
Subject: bug#18175: files.el: use mapc in (mapcar 'switch-to-buffer ...) 
Date: Sun, 03 Aug 2014 08:55:21 +0000
[Message part 1 (text/plain, inline)]
>>>>> Drew Adams <drew.adams <at> 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
[Message part 2 (text/x-diff, inline)]
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)

This bug report was last modified 10 years and 179 days ago.

Previous Next


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