Package: emacs;
Reported by: Dave Abrahams <dave <at> boostpro.com>
Date: Sat, 28 Jul 2012 20:55:01 UTC
Severity: normal
Found in version 24.1
Done: martin rudalics <rudalics <at> gmx.at>
Bug is archived. No further changes may be made.
View this message in rfc822 format
From: Dave Abrahams <dave <at> boostpro.com> To: Alp Aker <alptekin.aker <at> gmail.com> Cc: martin rudalics <rudalics <at> gmx.at>, 12081 <at> debbugs.gnu.org Subject: bug#12081: 24.1; buffer-predicate often not called Date: Sun, 29 Jul 2012 14:24:46 -0400
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 <at> 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
GNU bug tracking system
Copyright (C) 1999 Darren O. Benham,
1997,2003 nCipher Corporation Ltd,
1994-97 Ian Jackson.