bug#81614: 30.2; quit-window unexpectedly changes the width of window

Hi Martin,

martin rudalics <[email protected]> writes:
> 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.

I have tested the patch and it fixes the reproducer, thanks a lot for that.
I don't think it is urgent to fix it. For now I have hacked around this
in my init.el by

(add-hook 'quit-window-hook
            (defun +quit-prevent-horizontal-resize ()
              (let ((window (get-buffer-window)))
                (unless (window-combined-p window)
                  (when-let* ((qr (nth 1 (window-parameter window 'quit-restore)))
                              ((consp qr)))
                    (setf (nth 3 qr) nil))))))

which isn't perfect but works for scenarios in which I discovered the
problem so good enough in the short term.

> 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.

I think the ability to restore width can be useful for e.g. displaying a
temporary buffer that have lines at most 80 columns wide.

> Thanks for both report and patch, martin

Thanks for the diff! I think it is the better solution.

> [2. text/x-patch; quit-restore-window.diff]...

Best,
Rahguzar