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

[martin rudalics]

>> Why is showing the buffer visiting /tmp/xx bad in your scenario?  Can
>> you give a scenario where the present behavior really hurts you?  In
>> that case we can try to ignore such a buffer in `switch-to-prev-buffer'.
>
> Well, note that it's a regression:  replace-buffer-in-windows used to
> call other-window (via window-loop).

This doesn't explain why showing the buffer visiting /tmp/xx is bad
here.  If you show a buffer A in a window, then show a buffer B in that
window and subsequently kill B, the most natural thing is to show A
again in that window.  In particular so, if B was used to display some
temporary information.

> As for how it hurts not to check buffer predicates in
> swtich-to-prev-buffer:  Buffer predicates are supposed to provide a
> way of exercising some control over what buffers are automatically
> selected for display.

In Dave's scenario neither of the file visiting buffers was shown
automatically but under the user's control.

> But if kill-buffer doesn't respect buffer
> predicates, then there's not much point to setting up a buffer
> predicate at all:  why bother filtering buffers chosen for display, if
> the filter isn't respected by one of the most common ways in which a
> buffer is chosen for the user?

`other-buffer' is used in many places for different reasons, so it's by
no means obvious that it's about "filtering buffers chosen for display".
It has been used in `replace-buffer-in-windows' because there was no
better alternative.  And it might have been a good idea to not call this
parameter "buffer-predicate" in the first place but something more
indicative.  Also the manual text

> `buffer-predicate'
>      The buffer-predicate function for this frame.  The function
>      `other-buffer' uses this predicate (from the selected frame) to
>      decide which buffers it should consider, if the predicate is not
>      `nil'.  It calls the predicate with one argument, a buffer, once
>      for each buffer; if the predicate returns a non-`nil' value, it
>      considers that buffer.

is misleading: Neither `other-buffer' nor `replace-buffer-in-windows'
necessarily care about which frame is selected when they get called.
When killing a buffer `replace-buffer-in-windows' obviously has to act
on all windows on all frames.

> It seems like something like the following would restore the old 
functionality:

What's the reason for doing

> +      ;; If we reach this, then either: (1) we have a
> +      ;; candidate buffer that was skipped because it was already visible on
> +      ;; the frame, in which case we switch to it now, or (2) no candidate
> +      ;; was found, in which case we switch to *scratch*.
> +      (if visible
> +          (setq new-buffer visible)
> +        (setq new-buffer (get-buffer-create "*scratch*")))
> +      (set-window-buffer-start-and-point window new-buffer))
> +
[...]
> +      ;; If we reach this, then either: (1) we have a candidate buffer that
> +      ;; was skipped because it was already visible on the frame, in which
> +      ;; case we switch to it now, or (2) no candidate was found, in which
> +      ;; case we switch to *scratch*.
> +      (if visible
> +          (setq new-buffer visible)
> +        (setq new-buffer (get-buffer-create "*scratch*")))
> +      (set-window-buffer-start-and-point window new-buffer))
> +

IIUC these changes are not related to the issue we're discussing here.
Apart from that I see no problems applying your patch.

martin