bug#81614: 30.2; quit-window unexpectedly changes the width of window
martin rudalics via "Bug reports for GNU Emacs, the Swiss army knife of text editors" <[email protected]>
| Newsgroups | gmane.emacs.bugs |
|---|---|
| Message-ID | <[email protected]> |
>>> To reproduce starting with emacs -Q, evaluate the following code in
>>> scratch buffer:
>>>
>>> (progn
>>> (display-buffer (messages-buffer) '(display-buffer-in-direction
>>> (direction . right)))
>>> (display-buffer (get-buffer-create "*buffer-2*")
>>> '(display-buffer-below-selected))
>>> (display-buffer (get-buffer-create "*buffer-1*")
>>> '(display-buffer-same-window))
>>> (select-window (window-next-sibling)))
>>>
>>> and then do C-x w q twice. The second quit-window changes the width of
>>> the two windows.
>>>
>>> I think the responsible code is this snippet from 'quit-restore-window'
>>>
>>> (when (and (integerp (nth 3 quad))
>>> (if (window-combined-p window)
>>> (/= (nth 3 quad) (window-total-height window))
>>> (/= (nth 3 quad) (window-total-width window))))
>>> ;; Try to resize WINDOW to its old height but don't signal an
>>> ;; error.
>>> (condition-case nil
>>> (window-resize
>>> window
>>> (- (nth 3 quad) (if (window-combined-p window)
>>> (window-total-height window)
>>> (window-total-width window)))
>>> (window-combined-p window t))
>>> (error nil)))
>>>
>>> As far as I can tell by searching around in 'window.el' the
>>> 'quit-restore' parameter only ever stores the height of the window. This
>>> matches the description of 'quit-restore' in Info node (elisp) Quitting
>>> Windows. But the code above instead changes the width of the window if
>>> the window is not vertically combined.
To explain the background of this type with emacs -Q
M-x temp-buffer-resize-mode
and then C-h f set RET. By default Emacs makes a *Help* window on the
bottom of the frame and fits it to its buffer. If you now do
(display-buffer (messages-buffer) '(nil (inhibit-same-window . t)))
this replaces *Help* with *Messages* and makes that window half of the
size of the frame. If you now do C-x o C-x w q, this shows *Help* again
in that window and resizes it back. If you now do C-x o C-x w q again
this deletes the *Help* window.
So the net effect of 'temp-buffer-resize-mode' remains restricted to
temporary buffers. Buffers like *Messages* are displayed in normally
sized windows.
The scenario of Bug#81614 is to evaluate
(progn
(display-buffer (messages-buffer) '(display-buffer-in-direction
(direction . right)))
(display-buffer (get-buffer-create "*buffer-2*")
'(display-buffer-below-selected))
(display-buffer (get-buffer-create "*buffer-1*")
'(display-buffer-same-window))
(select-window (window-next-sibling)))
followed by typing C-x w q two times. Now after the first two
'display-buffer' calls the root window is a horizontal combination whose
window on the left is a vertical combination. The third
'display-buffer' call shows *buffer-1* instead of *scratch* and the
'quit-restore' parameter of that window becomes
(other (#<buffer *scratch*> 1 #<marker at 484 in *scratch*> 17) #<window 3 on *buffer-1*> #<buffer *buffer-1*>)
nil
where "other" means another buffer was shown in that window before,
*scratch* is the name of that buffer and "17" indicates the old height
of the window in canonical lines,
The scenario misfires because the first subsequent C-x w q deletes the
window of *buffer-2* which makes the window on *buffer-1*, which was
vertically combined when first showing *buffer-1* in it, horizontally
combined. The second C-x w q shows *scratch* again in that window
according to the 'quit-restore' parameter. But the subsequent resizing
step resizes the window horizontally to 17 columns because the initial
combination state was lost.
I also suppose that Bug#81614 provides a recipe for a behavior Dmitry
initially observed in Bug#78835 but was not able to reproduce reliably.
A fix is to remember the combination in a fifth subslot of the second
slot of the 'quit-restore' parameter and to resize the window iff the
value stored there matches the actual combination state. The fix also
no more copies the 'quit-restore' parameter in ‘split-window-below’ and
'split-window-right' to avoid a similar effect.
The attached fix is fairly trivial - it mostly changes the term "quad"
to "quint". So I consider it safe for the release branch including the
removal of copying the 'quit-restore' parameter during splitting. OTOH
this bug is quite old - reproducible with Emacs 27 at least - so we can
save it for Emacs 32 as well.
>> Subject: [PATCH] Don't change width in quit restore (bug#81614)
>>
>> * lisp/window.el (quit-restore-window): only resize
>> vertically combined windows
>> ---
>> lisp/window.el | 17 ++++++-----------
>> 1 file changed, 6 insertions(+), 11 deletions(-)
>>
>> diff --git a/lisp/window.el b/lisp/window.el
>> index fef2f3d4666..f3b33023f79 100644
>> --- a/lisp/window.el
>> +++ b/lisp/window.el
>> @@ -5481,19 +5481,14 @@ quit-restore-window
>> (eq (nth 3 quit-restore) buffer)))
>> ;; Show another buffer stored in quit-restore(-prev) parameter.
>> (when (and (integerp (nth 3 quad))
>> - (if (window-combined-p window)
>> - (/= (nth 3 quad) (window-total-height window))
>> - (/= (nth 3 quad) (window-total-width window))))
>> + (window-combined-p window)
>> + (/= (nth 3 quad) (window-total-height window)))
>> ;; Try to resize WINDOW to its old height but don't signal an
>> ;; error.
>> - (condition-case nil
>> - (window-resize
>> - window
>> - (- (nth 3 quad) (if (window-combined-p window)
>> - (window-total-height window)
>> - (window-total-width window)))
>> - (window-combined-p window t))
>> - (error nil)))
>> + (ignore-errors
>> + (window-resize
>> + window
>> + (- (nth 3 quad) (window-total-height window)))))
>> (set-window-dedicated-p window nil)
>> ;; Restore WINDOW's previous buffer, start and point position.
>> (set-window-buffer-start-and-point
>> --
>> 2.55.0
This patch does away with sizing windows back horizontally. I don't
think that sizing windows back horizontally is overly useful so I'm not
against using this as concept. But then we should never save a width in
the 'quit-restore' parameter in the first place which means to reduce
the
(if (window-combined-p window)
(window-total-height window)
(window-total-width window))
form in 'display-buffer-record-window' to
(window-total-height window)
In addition, we would have to check whether windows are vertically
combined when sizing back in both 'display-buffer-use-some-window' and
'display-buffer-use-least-recent-window' which are likely broken as well
when a width value was saved and the window used by these operations is
vertically combined (I'm too lazy to construct an example). Rahguzar,
can you include these three changes in your fix? I think it would then
be pretty safe for the release version too.
Thanks for both report and patch, martin
quit-restore-window.diff
(text/x-patch, 8.4 KB)
diff --git a/lisp/window.el b/lisp/window.el
index d958bfcd1e2..0625e76007b 100644
--- a/lisp/window.el
+++ b/lisp/window.el
@@ -5439,7 +5439,7 @@ quit-restore-window
(throw 'prev-buffer (car buf))))))
(dedicated (window-dedicated-p window))
(frame (window-frame window))
- quad entry reset-prev)
+ quint entry reset-prev)
(cond
;; First try to delete dedicated windows that are not side windows.
((and dedicated (not (eq dedicated 'side))
@@ -5469,27 +5469,28 @@ quit-restore-window
window nil (memq bury-or-kill '(kill killing))))
;; If the previously selected window is still alive, select it.
(window--quit-restore-select-window quit-restore-2 frame))
- ((or (and (listp (setq quad (nth 1 quit-restore-prev)))
- (buffer-live-p (car quad))
+ ((or (and (listp (setq quint (nth 1 quit-restore-prev)))
+ (buffer-live-p (car quint))
(eq (nth 3 quit-restore-prev) buffer)
;; Use selected window from quit-restore-prev.
(setq quit-restore-2 quit-restore-prev-2)
;; We want to reset quit-restore-prev only.
(setq reset-prev t))
- (and (listp (setq quad (nth 1 quit-restore)))
- (buffer-live-p (car quad))
+ (and (listp (setq quint (nth 1 quit-restore)))
+ (buffer-live-p (car quint))
(eq (nth 3 quit-restore) buffer)))
;; Show another buffer stored in quit-restore(-prev) parameter.
- (when (and (integerp (nth 3 quad))
+ (when (and (integerp (nth 3 quint))
+ (eq (nth 4 quint) (window-combined-p window))
(if (window-combined-p window)
- (/= (nth 3 quad) (window-total-height window))
- (/= (nth 3 quad) (window-total-width window))))
+ (/= (nth 3 quint) (window-total-height window))
+ (/= (nth 3 quint) (window-total-width window))))
;; Try to resize WINDOW to its old height but don't signal an
;; error.
(condition-case nil
(window-resize
window
- (- (nth 3 quad) (if (window-combined-p window)
+ (- (nth 3 quint) (if (window-combined-p window)
(window-total-height window)
(window-total-width window)))
(window-combined-p window t))
@@ -5497,7 +5498,7 @@ quit-restore-window
(set-window-dedicated-p window nil)
;; Restore WINDOW's previous buffer, start and point position.
(set-window-buffer-start-and-point
- window (nth 0 quad) (nth 1 quad) (nth 2 quad))
+ window (nth 0 quint) (nth 1 quint) (nth 2 quint))
;; Restore the 'side' dedicated flag as well.
(when (eq dedicated 'side)
(set-window-dedicated-p window 'side))
@@ -5939,16 +5940,6 @@ split-window
window (- (if new-parent 1.0 (window-normal-size window horizontal))
new-normal)))
- (unless horizontal
- (let ((quit-restore (window-parameter window 'quit-restore)))
- (when quit-restore
- (let ((quad (nth 1 quit-restore)))
- (when (and (listp quad) (integerp (nth 3 quad)))
- ;; When WINDOW has a 'quit-restore' parameter that
- ;; specifies a previous height to restore, remove that
- ;; - it does more harm than good now (Bug#78835).
- (setf (nth 3 quad) nil))))))
-
(let ((new (split-window-internal
window new-pixel-size side new-normal refer)))
(window--pixel-to-total frame horizontal)
@@ -6065,10 +6056,7 @@ split-window-below
(<= (window-start new-window) old-point)
(set-window-point new-window old-point)
(select-window new-window))))
- ;; Always copy quit-restore parameter in interactive use.
- (let ((quit-restore (window-parameter window-to-split 'quit-restore)))
- (when quit-restore
- (set-window-parameter new-window 'quit-restore quit-restore)))
+
new-window))
(defalias 'split-window-vertically 'split-window-below)
@@ -6106,10 +6094,6 @@ split-window-right
;; `split-window' would not signal an error here.
(error "Size of new window too small"))
(setq new-window (split-window window-to-split size t))
- ;; Always copy quit-restore parameter in interactive use.
- (let ((quit-restore (window-parameter window-to-split 'quit-restore)))
- (when quit-restore
- (set-window-parameter new-window 'quit-restore quit-restore)))
new-window))
(defalias 'split-window-horizontally 'split-window-right)
@@ -7042,14 +7026,16 @@ display-buffer-record-window
(set-window-parameter
window (if quit-restore 'quit-restore-prev 'quit-restore)
(list 'other
- ;; A quadruple of WINDOW's buffer, start, point and height.
+ ;; A quintuple of WINDOW's buffer, start, point, height
+ ;; and combination direction.
(list (current-buffer) (window-start window)
;; Preserve window-point-insertion-type (Bug#12855).
(copy-marker
(window-point window) window-point-insertion-type)
(if (window-combined-p window)
(window-total-height window)
- (window-total-width window)))
+ (window-total-width window))
+ (window-combined-p window))
(selected-window) buffer))))))
((eq type 'window)
;; WINDOW has been created on an existing frame.
@@ -9303,15 +9289,16 @@ display-buffer-use-some-window
(get-largest-window 0 nil not-this-window)))
(quit-restore (and (window-live-p window)
(window-parameter window 'quit-restore)))
- (quad (nth 1 quit-restore)))
+ (quint (nth 1 quit-restore)))
(when (window-live-p window)
;; If the window was used by `display-buffer' before, try to
;; resize it to its old height but don't signal an error.
- (when (and (listp quad)
- (integerp (nth 3 quad))
- (> (nth 3 quad) (window-total-height window)))
+ (when (and (listp quint)
+ (integerp (nth 3 quint))
+ (eq (nth 4 quint) (window-combined-p window))
+ (> (nth 3 quint) (window-total-height window)))
(condition-case nil
- (window-resize window (- (nth 3 quad) (window-total-height window)))
+ (window-resize window (- (nth 3 quint) (window-total-height window)))
(error nil)))
(prog1
@@ -9369,15 +9356,16 @@ display-buffer-use-least-recent-window
(let ((window (display-buffer--lru-window alist)))
(when (window-live-p window)
(let* ((quit-restore (window-parameter window 'quit-restore))
- (quad (nth 1 quit-restore)))
+ (quint (nth 1 quit-restore)))
;; If the window was used by `display-buffer' before, try to
;; resize it to its old height but don't signal an error.
- (when (and (listp quad)
- (integerp (nth 3 quad))
- (> (nth 3 quad) (window-total-height window)))
+ (when (and (listp quint)
+ (integerp (nth 3 quint))
+ (eq (nth 4 quint) (window-combined-p window))
+ (> (nth 3 quint) (window-total-height window)))
(condition-case nil
(window-resize
- window (- (nth 3 quad) (window-total-height window)))
+ window (- (nth 3 quint) (window-total-height window)))
(error nil)))
(prog1
(window--display-buffer buffer window 'reuse alist)
diff --git a/src/window.c b/src/window.c
index 022fb6a0d9f..6e96e9c3000 100644
--- a/src/window.c
+++ b/src/window.c
@@ -3412,16 +3412,16 @@ window_discard_buffer_from_window (Lisp_Object buffer, Lisp_Object window, bool
{
Lisp_Object quit_restore = window_parameter (w, Qquit_restore);
Lisp_Object quit_restore_prev = window_parameter (w, Qquit_restore_prev);
- Lisp_Object quad;
+ Lisp_Object quint;
if (EQ (buffer, Fnth (make_fixnum (3), quit_restore_prev))
- || (CONSP (quad = Fcar (Fcdr (quit_restore_prev)))
- && EQ (Fcar (quad), buffer)))
+ || (CONSP (quint = Fcar (Fcdr (quit_restore_prev)))
+ && EQ (Fcar (quint), buffer)))
Fset_window_parameter (window, Qquit_restore_prev, Qnil);
if (EQ (buffer, Fnth (make_fixnum (3), quit_restore))
- || (CONSP (quad = Fcar (Fcdr (quit_restore)))
- && EQ (Fcar (quad), buffer)))
+ || (CONSP (quint = Fcar (Fcdr (quit_restore)))
+ && EQ (Fcar (quint), buffer)))
{
Fset_window_parameter (window, Qquit_restore,
window_parameter (w, Qquit_restore_prev));