bug#48493: 28.0.50; quit-window doesn't work

[pillule <pillule@riseup.net>]

[-- Attachment #1: Type: text/plain, Size: 2134 bytes --]


pillule <pillule@riseup.net> writes:

> martin rudalics <rudalics@gmx.at> writes:
>
>> This is quite a weakness of the present mechanism and I think 
>> you got
>> it right.  To summarize your approach:
>>
>> - When we have to replace a buffer in a side window, that 
>> window's
>>   dedicated status is 'side', and some other buffer is found 
>>   that was
>>   shown there before (it's on the list of that window's 
>>   _previous_
>>   buffers) we show that other buffer in the window and make 
>>   sure   to
>>   restore the window's dedicated status to 'side'.
>>
>> - Otherwise delete the window.  Deleting the window is always 
>> possible
>>   and we have to make sure one thing - never show in a side 
>>   window a
>>   buffer that has not been shown before in that window.  This 
>>   rule
>>   should take care of the DOOM workaround.
>>
>> And users who override the behavior sketched here by setting 
>> the side
>> window's dedicated status to t should have the window deleted 
>> (since
>> that is always possible).
>>
>> Have I got it right?
>
> Yes.
> Maybe it is a step toward implementing the `soft' dedication 
> that is mentioned in the comments.
> Thanks you for the explanations.
>
>> We have to document it in the Elisp manual.
>
> Here another draft with the info manual changes:
>
> From what I have tested so far, there were more functions that 
> needed to preserve the side
> dedication. I put in my modeline a token for the window 
> dedication and it was quite useful to spot
> them (maybe not the clever way but worked). I also grepped into 
> window.el to see if I was missing
> something without more success.
>
> There is a change in the indentation of `quit-restore-window' 
> and I don't know if there is
> convention in such cases.

Sorry I forgot a last fix on `quit-restore-window' in the last 
message please consider instead this one :
(because switch-to-prev/next-buffer must return nil in case of no 
available buffer, we can use it in conditional and simplify the 
code, also the previous version was changing the behavior of some 
windows by deleting them when it was not necessary)


[-- Attachment #2: improve handling of side dedication when quitting windows --]
[-- Type: text/x-diff, Size: 21119 bytes --]

From 3fd3f1c0aa9175e28224d47f6625a442b8cf540d Mon Sep 17 00:00:00 2001
From: Trust me I am a doctor <pillule@riseup.net>
Date: Tue, 8 Jun 2021 11:20:16 +0200
Subject: [PATCH] Improve handling of side dedicated flag

Following the discussion on (Bug#48493), restore the dedicated side
flag of windows when a buffer change in the window, update the
documentation.

* doc/lispref/windows.texi
  (Buffers and Windows): mention the exception of side windows and
add a reference.
  (Buffer Display Action Alists): explicit that
`display-buffer-in-side-window' is dedicating to side by default.
  (Dedicated Windows): add the case (4) and explicit its meaning,
add a reference.
  (Displaying Buffers in Side Windows): move the
switch-to-(prev|next)-buffer paragraph into a new item to emphasize
the special meaning of dedication for side windows.

* lisp/window.el
  (set-window-buffer-start-and-point): restore side dedication.
  (switch-to-prev-buffer): corrige the return value that should be
nil instead of the same buffer in case of no changement.
  (switch-to-next-buffer): corrige the return value that should be
nil instead of the same buffer in case of no changement.
  (delete-windows-on): restore side dedication.
  (replace-buffer-in-windows): update the docstring, restore side
dedication.
  (quit-restore-window): rearrange the logic so hard dedicated windows
are eventually deleted first, restore the side dedication, in the
final case try to `switch-to-prev-buffer' before deleting a window
fix (Bug#48367)
---
 doc/lispref/windows.texi |  58 +++++++----
 lisp/window.el           | 210 ++++++++++++++++++++++-----------------
 2 files changed, 153 insertions(+), 115 deletions(-)

diff --git a/doc/lispref/windows.texi b/doc/lispref/windows.texi
index 44656c057a..601f59e650 100644
--- a/doc/lispref/windows.texi
+++ b/doc/lispref/windows.texi
@@ -2172,12 +2172,13 @@ Buffers and Windows
 the current buffer.
 
 The replacement buffer in each window is chosen via
-@code{switch-to-prev-buffer} (@pxref{Window History}).  Any dedicated
-window displaying @var{buffer-or-name} is deleted if possible
-(@pxref{Dedicated Windows}).  If such a window is the only window on its
-frame and there are other frames on the same terminal, the frame is
-deleted as well.  If the dedicated window is the only window on the only
-frame on its terminal, the buffer is replaced anyway.
+@code{switch-to-prev-buffer} (@pxref{Window History}).  With the
+exception of side windows, any dedicated window displaying
+@var{buffer-or-name} is deleted if possible (@pxref{Dedicated
+Windows}).  If such a window is the only window on its frame and there
+are other frames on the same terminal, the frame is deleted as well.
+If the dedicated window is the only window on the only frame on its
+terminal, the buffer is replaced anyway.
 @end deffn
 
 
@@ -2994,6 +2995,8 @@ Buffer Display Action Alists
 any window it creates as dedicated to its buffer (@pxref{Dedicated
 Windows}).  It does that by calling @code{set-window-dedicated-p} with
 the chosen window as first argument and the entry's value as second.
+Side windows are by default dedicated with the value @code{side}
+((@pxref{Side Window Options and Functions}).
 
 @vindex preserve-size@r{, a buffer display action alist entry}
 @item preserve-size
@@ -4042,18 +4045,19 @@ Dedicated Windows
 
    Functions supposed to remove a buffer from a window or a window from
 a frame can behave specially when a window they operate on is dedicated.
-We will distinguish three basic cases, namely where (1) the window is
+We will distinguish four basic cases, namely where (1) the window is
 not the only window on its frame, (2) the window is the only window on
-its frame but there are other frames on the same terminal left, and (3)
-the window is the only window on the only frame on the same terminal.
+its frame but there are other frames on the same terminal left, (3)
+the window is the only window on the only frame on the same terminal,
+and (4) the dedication's value is @code{side}
+(@pxref{Displaying Buffers in Side Windows}).
 
    In particular, @code{delete-windows-on} (@pxref{Deleting Windows})
-handles case (2) by deleting the associated frame and case (3) by
-showing another buffer in that frame's only window.  The function
+handles case (2) by deleting the associated frame and case (3) and (4)
+by showing another buffer in that frame's only window.  The function
 @code{replace-buffer-in-windows} (@pxref{Buffers and Windows}) which is
 called when a buffer gets killed, deletes the window in case (1) and
 behaves like @code{delete-windows-on} otherwise.
-@c FIXME: Does replace-buffer-in-windows _delete_ a window in case (1)?
 
    When @code{bury-buffer} (@pxref{Buffer List}) operates on the
 selected window (which shows the buffer that shall be buried), it
@@ -4316,6 +4320,26 @@ Displaying Buffers in Side Windows
 middle slot.  Hence, all windows on a specific side are ordered by their
 @code{slot} value.  If unspecified, the window is located in the middle
 of the specified side.
+
+
+@item dedicated
+The dedicated flag is not reserved to this function but have a
+slightly different meaning for side windows.  They receive it upon
+creation with the value @code{side}; it serves to prevent
+@code{display-buffer} to uses these windows with others action
+functions, and it persists across invocations of @code{quit-window},
+@code{kill-buffer}, @code{previous-buffer} and @code{next-buffer}
+(@pxref{note Window History}).  In particular, these commands will
+refrain from showing, in a side window, buffers that have not been
+displayed in that window before.  They will also refrain from having a
+normal, non-side window show a buffer that has been already displayed
+in a side window.  A notable exception to the latter rule occurs when
+an application, after displaying a buffer, resets that buffer’s local
+variables.  To override these rules and always delete a side window
+with @code{quit-window} or @code{kill-buffer}, and eventually prevent
+the use of @code{previous-buffer} and @code{next-buffer}, set this
+value to @code{t} or specify a value for
+@code{display-buffer-mark-dedicated}.
 @end table
 
 If you specify the same slot on the same side for two or more different
@@ -4336,16 +4360,6 @@ Displaying Buffers in Side Windows
 action.  Note also that @code{delete-other-windows} cannot make a side
 window the only window on its frame (@pxref{Deleting Windows}).
 
-   Once set up, side windows also change the behavior of the commands
-@code{switch-to-prev-buffer} and @code{switch-to-next-buffer}
-(@pxref{Window History}).  In particular, these commands will refrain
-from showing, in a side window, buffers that have not been displayed in
-that window before.  They will also refrain from having a normal,
-non-side window show a buffer that has been already displayed in a side
-window.  A notable exception to the latter rule occurs when an
-application, after displaying a buffer, resets that buffer's local
-variables.
-
 
 @node Side Window Options and Functions
 @subsection Side Window Options and Functions
diff --git a/lisp/window.el b/lisp/window.el
index fd1c617d6b..d3e3152cd0 100644
--- a/lisp/window.el
+++ b/lisp/window.el
@@ -4424,8 +4424,12 @@ set-window-buffer-start-and-point
 before was current this also makes BUFFER the current buffer."
   (setq window (window-normalize-window window t))
   (let ((selected (eq window (selected-window)))
-	(current (eq (window-buffer window) (current-buffer))))
+	(current (eq (window-buffer window) (current-buffer)))
+        (dedicated (window-dedicated-p window)))
     (set-window-buffer window buffer)
+    ;; restore the dedicated side flag
+    (when (eq dedicated 'side)
+      (set-window-dedicated-p window 'side))
     (when (and selected current)
       (set-buffer buffer))
     (when start
@@ -4559,11 +4563,11 @@ switch-to-prev-buffer
       ;; Scan WINDOW's previous buffers first, skipping entries of next
       ;; buffers.
       (dolist (entry (window-prev-buffers window))
-	(when (and (setq new-buffer (car entry))
+	(when (and (not (eq (car entry) old-buffer))
+                   (setq new-buffer (car entry))
 		   (or (buffer-live-p new-buffer)
 		       (not (setq killed-buffers
 				  (cons new-buffer killed-buffers))))
-		   (not (eq new-buffer old-buffer))
                    (or (null pred) (funcall pred new-buffer))
 		   ;; When BURY-OR-KILL is nil, avoid switching to a
 		   ;; buffer in WINDOW's next buffers list.
@@ -4726,11 +4730,11 @@ switch-to-next-buffer
       ;; Scan WINDOW's reverted previous buffers last (must not use
       ;; nreverse here!)
       (dolist (entry (reverse (window-prev-buffers window)))
-	(when (and (setq new-buffer (car entry))
+	(when (and (not (eq new-buffer (car entry)))
+                   (setq new-buffer (car entry))
 		   (or (buffer-live-p new-buffer)
 		       (not (setq killed-buffers
 				  (cons new-buffer killed-buffers))))
-		   (not (eq new-buffer old-buffer))
                    (or (null pred) (funcall pred new-buffer)))
           (if (switch-to-prev-buffer-skip-p skip window new-buffer)
 	      (setq skipped (or skipped new-buffer))
@@ -4957,9 +4961,10 @@ delete-windows-on
 	(all-frames (cond ((not frame) t) ((eq frame t) nil) (t frame))))
     (dolist (window (window-list-1 nil nil all-frames))
       (if (eq (window-buffer window) buffer)
-	  (let ((deletable (window-deletable-p window)))
+	  (let ((deletable (window-deletable-p window))
+                (dedicated (window-dedicated-p window)))
 	    (cond
-	     ((and (eq deletable 'frame) (window-dedicated-p window))
+	     ((and (eq deletable 'frame) dedicated)
 	      ;; Delete frame if and only if window is dedicated.
 	      (delete-frame (window-frame window)))
 	     ((eq deletable t)
@@ -4968,7 +4973,10 @@ delete-windows-on
 	     (t
 	      ;; In window switch to previous buffer.
 	      (set-window-dedicated-p window nil)
-	      (switch-to-prev-buffer window 'bury))))
+	      (switch-to-prev-buffer window 'bury)
+              ;; restore the dedicated side flag
+              (when (eq dedicated 'side)
+                (set-window-dedicated-p window 'side)))))
 	;; If a window doesn't show BUFFER, unrecord BUFFER in it.
 	(unrecord-window-buffer window buffer)))))
 
@@ -4977,10 +4985,10 @@ replace-buffer-in-windows
 BUFFER-OR-NAME may be a buffer or the name of an existing buffer
 and defaults to the current buffer.
 
-When a window showing BUFFER-OR-NAME is dedicated, that window is
-deleted.  If that window is the only window on its frame, the
-frame is deleted too when there are other frames left.  If there
-are no other frames left, some other buffer is displayed in that
+With the exception of side windows, when a window showing BUFFER-OR-NAME
+is dedicated, that window is deleted.  If that window is the only window
+on its frame, the frame is deleted too when there are other frames left.
+If there are no other frames left, some other buffer is displayed in that
 window.
 
 This function removes the buffer denoted by BUFFER-OR-NAME from
@@ -4989,10 +4997,14 @@ replace-buffer-in-windows
   (let ((buffer (window-normalize-buffer buffer-or-name)))
     (dolist (window (window-list-1 nil nil t))
       (if (eq (window-buffer window) buffer)
-	  (unless (window--delete window t t)
-	    ;; Switch to another buffer in window.
-	    (set-window-dedicated-p window nil)
-	    (switch-to-prev-buffer window 'kill))
+          ;; delete dedicated window that are not side windows
+          (let ((dedicated-side (eq (window-dedicated-p window) 'side)))
+            (when (or dedicated-side (not (window--delete window t t)))
+              ;; Switch to another buffer in window.
+              (set-window-dedicated-p window nil)
+              (if (switch-to-prev-buffer window 'kill)
+                  (and dedicated-side (set-window-dedicated-p window 'side))
+                (window--delete window nil 'kill))))
 	;; Unrecord BUFFER in WINDOW.
 	(unrecord-window-buffer window buffer)))))
 
@@ -5014,6 +5026,10 @@ quit-restore-window
 parameter to nil.  See Info node `(elisp) Quitting Windows' for
 more details.
 
+If WINDOW have the flag dedicated with the value t, always try to
+delete WINDOW, with the value side, restore that value when
+WINDOW is not deleted.
+
 Optional second argument BURY-OR-KILL tells how to proceed with
 the buffer of WINDOW.  The following values are handled:
 
@@ -5040,87 +5056,95 @@ quit-restore-window
                         (dolist (buf (window-prev-buffers window))
                           (unless (eq (car buf) buffer)
                             (throw 'prev-buffer (car buf))))))
+         (dedicated (window-dedicated-p window))
 	 quad entry)
-    (cond
-     ((and (not prev-buffer)
-	   (eq (nth 1 quit-restore) 'tab)
-	   (eq (nth 3 quit-restore) buffer))
-      (tab-bar-close-tab)
-      ;; If the previously selected window is still alive, select it.
-      (when (window-live-p (nth 2 quit-restore))
-	(select-window (nth 2 quit-restore))))
-     ((and (not prev-buffer)
-	   (or (eq (nth 1 quit-restore) 'frame)
-	       (and (eq (nth 1 quit-restore) 'window)
-		    ;; If the window has been created on an existing
-		    ;; frame and ended up as the sole window on that
-		    ;; frame, do not delete it (Bug#12764).
-		    (not (eq window (frame-root-window window)))))
-	   (eq (nth 3 quit-restore) buffer)
-	   ;; Delete WINDOW if possible.
-	   (window--delete window nil (eq bury-or-kill 'kill)))
-      ;; If the previously selected window is still alive, select it.
-      (when (window-live-p (nth 2 quit-restore))
-	(select-window (nth 2 quit-restore))))
-     ((and (listp (setq quad (nth 1 quit-restore)))
-	   (buffer-live-p (car quad))
-	   (eq (nth 3 quit-restore) buffer))
-      ;; Show another buffer stored in quit-restore 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))))
-	;; 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)))
-      (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))
-      ;; Deal with the buffer we just removed from WINDOW.
-      (setq entry (and (eq bury-or-kill 'append)
-		       (assq buffer (window-prev-buffers window))))
-      (when bury-or-kill
-	;; Remove buffer from WINDOW's previous and next buffers.
-	(set-window-prev-buffers
-	 window (assq-delete-all buffer (window-prev-buffers window)))
-	(set-window-next-buffers
-	 window (delq buffer (window-next-buffers window))))
-      (when entry
-	;; Append old buffer's entry to list of WINDOW's previous
-	;; buffers so it's less likely to get switched to soon but
-	;; `display-buffer-in-previous-window' can nevertheless find it.
-	(set-window-prev-buffers
-	 window (append (window-prev-buffers window) (list entry))))
-      ;; Reset the quit-restore parameter.
-      (set-window-parameter window 'quit-restore nil)
-      ;; Select old window.
-      (when (window-live-p (nth 2 quit-restore))
-	(select-window (nth 2 quit-restore))))
-     (t
-      ;; Show some other buffer in WINDOW and reset the quit-restore
-      ;; parameter.
-      (set-window-parameter window 'quit-restore nil)
-      ;; Make sure that WINDOW is no more dedicated.
-      (set-window-dedicated-p window nil)
-      (unless (switch-to-prev-buffer window bury-or-kill)
-        ;; Delete WINDOW if there is no previous buffer (Bug#48367).
-        (window--delete window nil (eq bury-or-kill 'kill)))))
+    (or ;; first try to delete dedicated windows that are not side windows
+     (and dedicated (not (eq dedicated 'side))
+          (window--delete window 'dedicated (eq bury-or-kill 'kill)))
+     (cond
+       ((and (not prev-buffer)
+             (eq (nth 1 quit-restore) 'tab)
+             (eq (nth 3 quit-restore) buffer))
+        (tab-bar-close-tab)
+        ;; If the previously selected window is still alive, select it.
+        (when (window-live-p (nth 2 quit-restore))
+          (select-window (nth 2 quit-restore))))
+       ((and (not prev-buffer)
+             (or (eq (nth 1 quit-restore) 'frame)
+                 (and (eq (nth 1 quit-restore) 'window)
+                      ;; If the window has been created on an existing
+                      ;; frame and ended up as the sole window on that
+                      ;; frame, do not delete it (Bug#12764).
+                      (not (eq window (frame-root-window window)))))
+             (eq (nth 3 quit-restore) buffer)
+             ;; Delete WINDOW if possible.
+             (window--delete window nil (eq bury-or-kill 'kill)))
+        ;; If the previously selected window is still alive, select it.
+        (when (window-live-p (nth 2 quit-restore))
+          (select-window (nth 2 quit-restore))))
+       ((and (listp (setq quad (nth 1 quit-restore)))
+             (buffer-live-p (car quad))
+             (eq (nth 3 quit-restore) buffer))
+        ;; Show another buffer stored in quit-restore 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))))
+          ;; 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)))
+        (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))
+        ;; and restore the dedicated side flag
+        (when (eq dedicated 'side) (set-window-dedicated-p window 'side))
+        ;; Deal with the buffer we just removed from WINDOW.
+        (setq entry (and (eq bury-or-kill 'append)
+                         (assq buffer (window-prev-buffers window))))
+        (when bury-or-kill
+          ;; Remove buffer from WINDOW's previous and next buffers.
+          (set-window-prev-buffers
+           window (assq-delete-all buffer (window-prev-buffers window)))
+          (set-window-next-buffers
+           window (delq buffer (window-next-buffers window))))
+        (when entry
+          ;; Append old buffer's entry to list of WINDOW's previous
+          ;; buffers so it's less likely to get switched to soon but
+          ;; `display-buffer-in-previous-window' can nevertheless find it.
+          (set-window-prev-buffers
+           window (append (window-prev-buffers window) (list entry))))
+        ;; Reset the quit-restore parameter.
+        (set-window-parameter window 'quit-restore nil)
+        ;; Select old window.
+        (when (window-live-p (nth 2 quit-restore))
+          (select-window (nth 2 quit-restore))))
+       (t
+        ;; Show some other buffer in WINDOW and reset the quit-restore
+        ;; parameter.
+        (set-window-parameter window 'quit-restore nil)
+        ;; Make sure that WINDOW is no more dedicated.
+        (set-window-dedicated-p window nil)
+        ;; (Bug#48367) try to swith to a previous buffer
+        ;; delete the window only if it is not possible
+        (if (switch-to-prev-buffer window bury-or-kill)
+            (set-window-dedicated-p window 'side)
+          (window--delete window nil (eq bury-or-kill 'kill))))))
 
     ;; Deal with the buffer.
     (cond
-     ((not (buffer-live-p buffer)))
-     ((eq bury-or-kill 'kill)
-      (kill-buffer buffer))
-     (bury-or-kill
-      (bury-buffer-internal buffer)))))
+      ((not (buffer-live-p buffer)))
+      ((eq bury-or-kill 'kill)
+       (kill-buffer buffer))
+      (bury-or-kill
+       (bury-buffer-internal buffer)))))
 
 (defun quit-window (&optional kill window)
   "Quit WINDOW and bury its buffer.
-- 
2.20.1


[-- Attachment #3: Type: text/plain, Size: 5 bytes --]



--