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

From: pillule <[email-masked]>
Date: Wed, 09 Jun 2021 14:34:54 +0200

Eli Zaretskii <[email-masked]> writes:

>> From: pillule <[email-masked]>
>> Date: Tue, 08 Jun 2021 01:23:11 +0200
>> Cc: pillule <[email-masked]>, Sujith Manoharan 
>> <[email-masked]>,
>>  [email-masked]
>>
>> > We have to document it in the Elisp manual.
>>
>> Here another draft with the info manual changes:
>
> Thanks.  Once again, I leave it to Martin and others to comment 
> on
> most of the essence of the patch, and provide here only few 
> minor
> nits:
>
>> * 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.
>
> Again, this log message is not formatted according to our rules. 
> It
> should look like this:
>
> * doc/lispref/windows.texi (Buffers and Windows): Mention the
> exception of side windows and add a reference.
> (Buffer Display Action Alists): Say explicitly that
> 'display-buffer-in-side-window' is dedicating to side by 
> default.
> (Dedicated Windows): Add case (4) and explain its meaning, add
> a reference.
> (Displaying Buffers in Side Windows): Move the paragraph about
> 'switch-to-(prev|next)-buffer' into a new item to emphasize the
> special meaning of dedication for side windows.
>
> Note that I also fixed the wording a bit, and that our 
> conventions for
> quoting in log entries is 'like this'.
>
>>  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
>    ^^^^^^^^^^^^^^^^^^^^^^^^^
> Here' it is quite important that the reader understands what are 
> "side
> windows", otherwise he/she will not understand the exception.
> However, "side window" was not yet described in the manual by 
> this
> point, and its description is not in this section.  In these 
> cases,
> always include a cross-reference to where the term is described, 
> like
> this:
>
>   With the exception of side windows (@pxref{Side Windows}), any 
>   ...
>
>>     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)
>                                                          ^^^^
> "cases", plural.
>

Here note that I suppressed the comment

@c FIXME: Does replace-buffer-in-windows _delete_ a window in case 
(1)?

Because yes, it is the case.

>> +@item dedicated
>> +The dedicated flag is not reserved to this function but have a
>                                                       ^    ^^^^
> A comma is missing before "but".  Also, please use "has" instead 
> of
> "have".
>
>> +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
>                          ^^^^^^^
> "to use"
>
>> +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
>            ^^^^^^^^^^^^^^^^^^^
> No need for "note" here.

Thanks.  Got them.

martin rudalics <[email-masked]> writes:

> Thanks for the patches.  Please follow ELi's suggestions first, 
> I'll
> then comment on the manual changes.
>
> -	(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))
>
> Here you could use the 'dedicated-side' solution you used below 
> in
> `replace-buffer-in-windows'.

ok

> +    (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))))
>
> What's wrong with putting the first disjunct into the 
> conditional as in
> the below?  In general, always try to avoid larger indentation 
> changes -
> they can make forensics cumbersome while bisecting.
>
>      (cond
>       ;; 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))))
>       ((and (not prev-buffer)
>              (eq (nth 1 quit-restore) 'tab)
>              (eq (nth 3 quit-restore) buffer))

The difference is a window dedicated with flag t may not be 
deletable, and in this case, we want it
to pass through the others conditionals branch of 
quit-restore-window, so it can try to use the
'quit-restore parameter, close the tab or to fallback in t, etc.
Explaining it makes me thing I could use 'window-deletable-p' in 
its conditional and ...
I guess, problem solved

In this revision I also restored the use of (select-window (nth 2 
quit-restore)) on the branch t.

> BTW, how's your paperwork process proceeding?

I am currently exchanging with the clerk to complete it.

>From 39fc2754de267cba77dcb9c8c567fc82bdcd17f9 Mon Sep 17 00:00:00 2001
From: Trust me I am a doctor <[email-masked]>
Date: Wed, 9 Jun 2021 13:43:21 +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): Say explicitly that
'display-buffer-in-side-window' is dedicating to side by default.
(Dedicated Windows): Add case (4) and explain its meaning, add
a reference.
(Displaying Buffers in Side Windows): Move the paragraph about
'switch-to-(prev|next)-buffer' 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): Correct the return value that should be
nil instead of the same buffer in case of no changement.
(switch-to-next-buffer): Correct 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           | 64 ++++++++++++++++++++++++++++------------
 2 files changed, 81 insertions(+), 41 deletions(-)

diff --git a/doc/lispref/windows.texi b/doc/lispref/windows.texi
index 44656c057a..3da206cd43 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 (@pxref{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 cases (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 has 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 use 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}.
+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..f843aead24 100644
--- a/lisp/window.el
+++ b/lisp/window.el
@@ -4424,8 +4424,10 @@ 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-side (eq (window-dedicated-p window) 'side)))
     (set-window-buffer window buffer)
+    (and dedicated-side (set-window-dedicated-p window 'side))
     (when (and selected current)
       (set-buffer buffer))
     (when start
@@ -4559,11 +4561,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 +4728,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 +4959,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 +4971,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 +4983,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 +4995,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 +5024,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,8 +5054,14 @@ 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
+     ;; first try to delete dedicated windows that are not side windows
+     ((and dedicated
+           (not (eq dedicated 'side))
+           (window-deletable-p window))
+      (window--delete window 'dedicated (eq bury-or-kill 'kill)))
      ((and (not prev-buffer)
 	   (eq (nth 1 quit-restore) 'tab)
 	   (eq (nth 3 quit-restore) buffer))
@@ -5084,6 +5104,8 @@ quit-restore-window
       ;; 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 side dedicated 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))))
@@ -5110,9 +5132,13 @@ quit-restore-window
       (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)))))
+      ;; (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))
+        (when (window-live-p (nth 2 quit-restore))
+          (select-window (nth 2 quit-restore))))))
 
     ;; Deal with the buffer.
     (cond
-- 
2.20.1

--