Revamping bs.el

[Juanma Barranquero <lekktu@gmail.com>]

[-- Attachment #1.1: Type: text/plain, Size: 3167 bytes --]

As a long-time user of bs.el, I've often been quite frustrated by some of
its limitations. It shows its age here and there, but mainly in two aspects:

- It uses kludgy code to save & restore window configurations to set the
frame back to a "normal" state after exiting; that code just doesn't play
nice with the newer display-buffer-alist window setup.

- It doesn't use tabulated-list-mode and instead creates its own clunky
"header line", sort functions, etc. One example where that goes wrong is
when the current bs-configuration wants to show more than
`bs-max-window-height' buffers; in this case, it is perfectly possible to
just move along the list until you push the "header line" out of the window.

So I'd like to work on this on master, for the 30.1 release.

For the first problem, the attached patch makes bs-show to use
pop-to-buffer, and adds a bs-default-action-list customizable option,
preloaded with a value that imitates the current, non-patched behavior,
while allowing setting display-buffer-alist, or the customizable option, to
further adapt bs to modern practices. The code also removes one function
and one variable, which I haven't obsoleted because they're intern.

Now, in my tests the behavior of bs-show has not changed (except for a tiny
detail where the patched behavior is, I think, superior to the old one**).
However, I'm pretty sure there will be some bugs lurking. As this is a
small change, easily reverted, and for a quite non-critical piece of Emacs,
I'd prefer to install the patch ASAP in master and let the bug reports
come, instead of trying to develop it in a feature branch that nobody will
fetch and test (and understandably so).

--------------
**The difference is that, with the old code,

emacs -Q
C-x 2
M-x bs-show <ret>

makes the bottom window grow while the *bs-selection* buffer is active
because the bs window is created at half the height of the top window, and
then shrunk. In my patch, I use window-combination-limit to force the top
window and the new bs window to be a combination group so their combined
height doesn't change.
--------------

Now, the second problem (deriving bs-mode from tabulated-list-mode) is not
as straightforward. Using the tabulated-list machinery to populate
*buffer-selection*, and create the header-line on the fly, is of course
easy enough. The problems come from the impedance mismatch between the
extremely column-oriented sorting machinery of tabulated-list-mode, vs the
freedom of bs sort functions that are not so limited. A few weeks ago I
spent a couple of days looking at the issue and came to the conclusion that
making bs work with tabulated-list-mode would require adding some
functionality to the later, so it can better communicate with its derived
mode and be aware that it has sorted the data in... "unexpected ways".

So, at this point,

1) Is there some interest (or, at the very least, indifference ;-) in my
revamping bs.el to work better with display-buffer-alist?
2) If so, it's ok to install the change in master?

Once this is settled, we can discuss whether the second part of the rework
is worth pursuing or not, and if so, what would be necessary.

[-- Attachment #1.2: Type: text/html, Size: 3726 bytes --]

[-- Attachment #2: 0001-lisp-bs.el-Adapt-to-modern-display-buffer-alist-wind.patch --]
[-- Type: application/octet-stream, Size: 5158 bytes --]

From 298ae623055175d198f16fecd71760eef113f3b2 Mon Sep 17 00:00:00 2001
From: Juanma Barranquero <lekktu@gmail.com>
Date: Tue, 6 Dec 2022 02:22:06 +0100
Subject: [PATCH] * lisp/bs.el: Adapt to modern display-buffer-alist window
 setup

* lisp/bs.el (bs--window-config-coming-from): Delete.
(bs-default-action-list): New user customizable option.
(bs--restore-window-config): Delete.
(bs-kill, bs-select, bs-select-other-window)
(bs-select-other-frame): Use `quit-window' instead.
(bs--show-with-configuration): Use `pop-to-buffer' to display
the "*buffer-selection*" buffer.
---
 lisp/bs.el | 58 ++++++++++++++++++++++++------------------------------
 1 file changed, 26 insertions(+), 32 deletions(-)

diff --git a/lisp/bs.el b/lisp/bs.el
index 1fd31fb3b8..289ede6819 100644
--- a/lisp/bs.el
+++ b/lisp/bs.el
@@ -420,9 +420,6 @@ bs--show-all
 Non-nil means to show all buffers.  Otherwise show buffers
 defined by current configuration `bs-current-configuration'.")
 
-(defvar bs--window-config-coming-from nil
-  "Window configuration before starting Buffer Selection Menu.")
-
 (defvar bs--intern-show-never "^ \\|\\*buffer-selection\\*"
   "Regular expression specifying which buffers never to show.
 A buffer whose name matches this regular expression will never be
@@ -491,7 +488,23 @@ bs-mode-map
   "<mouse-2>" #'bs-mouse-select
   "<mouse-3>" #'bs-mouse-select-other-frame)
 
-;; ----------------------------------------------------------------------
+(defcustom bs-default-action-list '((display-buffer-reuse-window
+				     display-buffer-below-selected)
+				    (window-height . bs-max-window-height))
+  "Default action list for showing the '*bs-selection*' buffer.
+
+This list will be passed to `pop-to-buffer' as its ACTION argument.
+It should be a cons cell (FUNCTIONS . ALIST), where FUNCTIONS is
+an action function or a list of action functions and ALIST is an
+action alist.  Each such action function should accept two
+arguments: a buffer to display and an alist of the same form as
+ALIST.  See ‘display-buffer’ for details."
+  :type display-buffer--action-custom-type
+  :risky t
+  :version "30.1"
+  :group 'bs)
+
+; ----------------------------------------------------------------------
 ;; Functions
 ;; ----------------------------------------------------------------------
 
@@ -668,20 +681,11 @@ bs-mode
   (add-hook 'kill-buffer-hook 'bs--remove-hooks nil t)
   (add-hook 'change-major-mode-hook 'bs--remove-hooks nil t))
 
-(defun bs--restore-window-config ()
-  "Restore window configuration on the current frame."
-  (when bs--window-config-coming-from
-    (let ((frame (selected-frame)))
-      (unwind-protect
-	   (set-window-configuration bs--window-config-coming-from)
-	(select-frame frame)))
-    (setq bs--window-config-coming-from nil)))
-
 (defun bs-kill ()
   "Let buffer disappear and reset window configuration."
   (interactive)
   (bury-buffer (current-buffer))
-  (bs--restore-window-config))
+  (quit-window))
 
 (defun bs-abort ()
   "Ding and leave Buffer Selection Menu without a selection."
@@ -742,7 +746,7 @@ bs-select
   (interactive)
   (let ((buffer (bs--current-buffer)))
     (bury-buffer (current-buffer))
-    (bs--restore-window-config)
+    (quit-window)
     (switch-to-buffer buffer)
     (when bs--marked-buffers
       ;; Some marked buffers for selection
@@ -765,7 +769,7 @@ bs-select-other-window
   (interactive)
   (let ((buffer (bs--current-buffer)))
     (bury-buffer (current-buffer))
-    (bs--restore-window-config)
+    (quit-window)
     (switch-to-buffer-other-window buffer)))
 
 (defun bs-tmp-select-other-window ()
@@ -781,7 +785,7 @@ bs-select-other-frame
   (interactive)
   (let ((buffer (bs--current-buffer)))
     (bury-buffer (current-buffer))
-    (bs--restore-window-config)
+    (quit-window)
     (switch-to-buffer-other-frame buffer)))
 
 (defun bs-mouse-select-other-frame (event)
@@ -1438,21 +1442,11 @@ bs--show-with-configuration
       ;; Only when not in buffer *buffer-selection*
       ;; we have to set the buffer we started the command
       (setq bs--buffer-coming-from (current-buffer)))
-    (let ((liste (bs-buffer-list))
-	  (active-window (get-window-with-predicate
-			  (lambda (w)
-			    (string= (buffer-name (window-buffer w))
-				     "*buffer-selection*"))
-			  nil (selected-frame))))
-      (if active-window
-	  (select-window active-window)
-	(bs--restore-window-config)
-	(setq bs--window-config-coming-from (current-window-configuration))
-	(when (> (window-height) 7)
-          ;; Errors would mess with the window configuration (bug#10882).
-          (ignore-errors (select-window (split-window-below)))))
-      (bs-show-in-buffer liste)
-      (bs-message-without-log "%s" (bs--current-config-message)))))
+    (let ((window-combination-limit 'window-size))
+      (pop-to-buffer (get-buffer-create "*buffer-selection*")
+		     bs-default-action-list))
+    (bs-show-in-buffer (bs-buffer-list))
+    (bs-message-without-log "%s" (bs--current-config-message))))
 
 (defun bs--configuration-name-for-prefix-arg (prefix)
   "Convert prefix argument PREFIX to a name of a buffer configuration.
-- 
2.38.1.windows.1