Subject: bug#12081: 24.1; buffer-predicate often not called

From: Alp Aker <[email-masked]>
Date: Tue, 31 Jul 2012 02:59:08 -0400

On Mon, Jul 30, 2012 at 1:42 PM, martin rudalics <[email-masked]> wrote:
> When `replace-buffer-in-windows' fails to replace the buffer,
> replace_buffer_in_windows_safely will kick in and automatically do what
> you mean.

Yes, I realized that about kill_buffer; my concern was whether other
users of replace-buffer-in-windows should need to provide fallback
behavior.  I'll defer to your judgment :)

So, would something like the following patch be ok (after adding
appropriate documentation changes)?  Note that with these changes,
there might still be some potentially odd behavior.  Here's a
contrived example.  From Emacs -Q, evaluating the following in
*scratch*:

(progn
  (set-frame-parameter nil 'buffer-predicate (lambda (x) nil))
  (switch-to-buffer (get-buffer-create "foo"))
  (switch-to-prev-buffer))

will leave buffer "foo" displayed in the selected window, but the
return value of of `switch-to-prev-buffer' will be #<buffer
*scratch*>, because `next-buffer' was assigned during the scan of the
previous buffers even though we did not switch to it.  Is such a
misleading return value ok?  (I realize this could be a considered a
variant of my earlier concern, which you argued was misplaced, but I
thought I should mention it nonetheless.)

=== modified file 'lisp/window.el'
--- lisp/window.el	2012-07-18 10:02:54 +0000
+++ lisp/window.el	2012-07-31 03:10:31 +0000
@@ -2679,6 +2679,7 @@
 	 (old-buffer (window-buffer window))
 	 ;; Save this since it's destroyed by `set-window-buffer'.
 	 (next-buffers (window-next-buffers window))
+         (pred (frame-parameter frame 'buffer-predicate))
 	 entry new-buffer killed-buffers visible)
     (when (window-dedicated-p window)
       (error "Window %s is dedicated to buffer %s" window old-buffer))
@@ -2692,6 +2693,7 @@
 		       (not (setq killed-buffers
 				  (cons new-buffer killed-buffers))))
 		   (not (eq new-buffer old-buffer))
+                   (or (null pred) (funcall pred new-buffer))
                    (or bury-or-kill
 		       (not (memq new-buffer next-buffers))))
 	  (if (and (not switch-to-visible-buffer)
@@ -2713,6 +2715,7 @@
 	(when (and (buffer-live-p buffer)
 		   (not (eq buffer old-buffer))
 		   (not (eq (aref (buffer-name buffer) 0) ?\s))
+                   (or (null pred) (funcall pred buffer))
 		   (or bury-or-kill (not (memq buffer next-buffers))))
 	  (if (get-buffer-window buffer frame)
 	      ;; Try to avoid showing a buffer visible in some other window.
@@ -2731,6 +2734,7 @@
 			 (not (setq killed-buffers
 				    (cons buffer killed-buffers))))
 		     (not (eq buffer old-buffer))
+                     (or (null pred) (funcall pred buffer))
 		     (setq entry (assq buffer (window-prev-buffers window))))
 	    (setq new-buffer buffer)
 	    (set-window-buffer-start-and-point
@@ -2773,6 +2777,7 @@
 	 (frame (window-frame window))
 	 (old-buffer (window-buffer window))
 	 (next-buffers (window-next-buffers window))
+         (pred (frame-parameter frame 'buffer-predicate))
 	 new-buffer entry killed-buffers visible)
     (when (window-dedicated-p window)
       (error "Window %s is dedicated to buffer %s" window old-buffer))
@@ -2784,6 +2789,7 @@
 		       (not (setq killed-buffers
 				  (cons buffer killed-buffers))))
 		   (not (eq buffer old-buffer))
+                   (or (null pred) (funcall pred buffer))
 		   (setq entry (assq buffer (window-prev-buffers window))))
 	  (setq new-buffer buffer)
 	  (set-window-buffer-start-and-point
@@ -2794,6 +2800,7 @@
       (dolist (buffer (buffer-list frame))
 	(when (and (buffer-live-p buffer) (not (eq buffer old-buffer))
 		   (not (eq (aref (buffer-name buffer) 0) ?\s))
+                   (or (null pred) (funcall pred buffer))
 		   (not (assq buffer (window-prev-buffers window))))
 	  (if (get-buffer-window buffer frame)
 	      ;; Try to avoid showing a buffer visible in some other window.
@@ -2808,6 +2815,7 @@
 		   (or (buffer-live-p new-buffer)
 		       (not (setq killed-buffers
 				  (cons new-buffer killed-buffers))))
+                   (or (null pred) (funcall pred new-buffer))
 		   (not (eq new-buffer old-buffer)))
 	  (if (and (not switch-to-visible-buffer)
 		   (get-buffer-window new-buffer frame))