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

[Dave Abrahams]

on Sun Jul 29 2012, Alp Aker <alptekin.aker-AT-gmail.com> wrote:

> On Sun, Jul 29, 2012 at 1:08 PM, martin rudalics <rudalics@gmx.at> wrote:
>> I see.  But `kill-buffer' calls `replace-buffer-in-windows' which
>> doesn't call `other-buffer'.  Only if the buffer to be killed is still
>> current after that, `kill-buffer' calls `other-buffer'.  In the scenario
>> above it is not called.
>>
>> 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).
>
> 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.  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?

Which is what I meant by "this renders the buffer-predicate
not-very-useful."

Thanks for explaining all that for me, Alp, and for your suggested fix.
As for my use-case, it's all in
https://github.com/dabrahams/workgroups.el/commit/88a605d2a6ba47ea7a918ee8650441317826cddb
(Workgroups with buffer affinity).

>
> It seems like something like the following would restore the old 
> functionality:
>
> === modified file 'lisp/window.el'
> --- lisp/window.el    2012-07-18 10:02:54 +0000
> +++ lisp/window.el    2012-07-29 16:59:18 +0000
> @@ -2679,10 +2679,12 @@
>        (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))
>
> +    (unless
>      (catch 'found
>        ;; Scan WINDOW's previous buffers first, skipping entries of next
>        ;; buffers.
> @@ -2692,6 +2694,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 +2716,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,16 +2735,22 @@
>                        (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
>            window new-buffer (nth 1 entry) (nth 2 entry))
> -         (throw 'found t))))
> -
> -      ;; Show a buffer visible in another window.
> -      (when visible
> -     (setq new-buffer visible)
> -     (set-window-buffer-start-and-point window new-buffer)))
> +         (throw 'found t)))))
> +
> +      ;; 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 bury-or-kill
>       ;; Remove `old-buffer' from WINDOW's previous and (restored list
> @@ -2773,10 +2783,12 @@
>        (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))
>
> +    (unless
>      (catch 'found
>        ;; Scan WINDOW's next buffers first.
>        (dolist (buffer next-buffers)
> @@ -2784,6 +2796,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 +2807,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 +2822,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))
> @@ -2816,12 +2831,17 @@
>               (setq visible new-buffer))
>           (set-window-buffer-start-and-point
>            window new-buffer (nth 1 entry) (nth 2 entry))
> -         (throw 'found t))))
> -
> -      ;; Show a buffer visible in another window.
> -      (when visible
> -     (setq new-buffer visible)
> -     (set-window-buffer-start-and-point window new-buffer)))
> +         (throw 'found t)))))
> +
> +      ;; 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))
> +
>
>      ;; Remove `new-buffer' from and restore WINDOW's next buffers.
>      (set-window-next-buffers window (delq new-buffer next-buffers))

-- 
Dave Abrahams
BoostPro Computing                  Software Development        Training
http://www.boostpro.com             Clang/LLVM/EDG Compilers  C++  Boost