Subject: bug#66394: 29.1; Make register-read-with-preview more useful

From: Thierry Volpiatto <[email-masked]>
Date: Mon, 18 Dec 2023 05:15:09 +0000

Stefan Monnier <[email-masked]> writes:

>> From 6a15a8b7b795b8377ef3537dc7bbd5ba26c0e159 Mon Sep 17 00:00:00 2001
>> From: Thierry Volpiatto <[email-masked]>
>> Date: Thu, 14 Dec 2023 16:23:26 +0100
>> Subject: [PATCH 3/4] Save and restore win confs after register commands
>>
>> When several windows are open, the register preview may eat other
>> windows and break the current window configuration.
>>
>> * lisp/register.el (register-read-with-preview-fancy): Do it.
>> ---
>>  lisp/register.el | 2 ++
>>  1 file changed, 2 insertions(+)
>>
>> diff --git a/lisp/register.el b/lisp/register.el
>> index 8f0c6a7105d..1fadbe81056 100644
>> --- a/lisp/register.el
>> +++ b/lisp/register.el
>> @@ -473,6 +473,7 @@ display such a window regardless."
>>           (buffer1 "*Register quick preview*")
>>           (buf (if register-use-preview buffer buffer1))
>>           (pat "")
>> +         (winconf (current-window-configuration))
>>           (map (let ((m (make-sparse-keymap)))
>>                  (set-keymap-parent m minibuffer-local-map)
>>                  m))
>> @@ -573,6 +574,7 @@ display such a window regardless."
>>                        nil "No register specified")
>>             (string-to-char result))
>>        (when timer (cancel-timer timer))
>> +      (set-window-configuration winconf)
>>        (let ((w (get-buffer-window buf)))
>>          (and (window-live-p w) (delete-window w)))
>>        (and (get-buffer buf) (kill-buffer buf)))))
>
> While save&restore of window configuration often works well, it
> misbehaves in various cases (such as when the buffer is displayed in
> another frame, which case case `set-window-configuration` will do you
> no good).

Yes, I thought this would be enough, but no, there is as well another
problem in register-preview and register-preview-1 with this code which
is as well not working well if there is multiple windows.

      (with-current-buffer-window
        buffer
        (cons 'display-buffer-below-selected
	      '((window-height . fit-window-to-buffer)
	        (preserve-size . (nil . t))))

[...]

At least it should use display-buffer-at-bottom, but I guess the best
would be to make this configurable in one way or the other.

>
> The official way to undo a `display-buffer` is with
> `quit-(restore-)window`.  See (info "(elisp)Quitting Windows").

Yes, this is not what the current code is using, see the last unwind
form of register-read-with-preview-basic/fancy.

But as Dmitri said, it is not bugs introduced by these patches, I
thought it would be simple to fix while we were at it but no.  I guess I
will follow Dmitri advice and not fix this in this serie of patches.
Perhaps we could make another bug report once these patches are applied
(I will remove the patch doing save/restore win conf) ?
 
-- 
Thierry