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

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

[Sujith Manoharan]

quit-window doesn't work properly with the master branch.

To see the issue with emacs -Q, in any git repo:

* C-x v L
* C-x 1 (if the screen is split).
* Hitting 'q' now doesn't close the window.

Looks like commit 0a681590268a4039f95a5a919b6b6d4f4e880d4c is
the problem.


In GNU Emacs 28.0.50 (build 1, x86_64-pc-linux-gnu, X toolkit, Xaw scroll bars)
 of 2021-05-18 built on comp-lnx
Repository revision: aff87e5d04de9ac59359ed29ca59483fc10d31ea
Repository branch: master
Windowing system distributor 'The X.Org Foundation', version 11.0.12011000
System Description: Arch Linux

Configured using:
 'configure --prefix=/usr/local --enable-link-time-optimization
 --with-sound=no --without-libsystemd --without-harfbuzz
 --without-m17n-flt --without-libotf --without-lcms2
 --with-x-toolkit=lucid --without-dbus --without-gsettings
 --without-selinux --without-modules --without-gpm --without-cairo
 'CFLAGS=-O2 -march=native''

Configured features:
ACL FREETYPE GIF GLIB GMP GNUTLS JPEG JSON LIBXML2 NOTIFY INOTIFY
PDUMPER PNG RSVG SECCOMP THREADS TIFF TOOLKIT_SCROLL_BARS X11 XDBE XFT
XIM XPM LUCID ZLIB

Important settings:
  value of $LANG: en_IN.UTF-8
  locale-coding-system: utf-8-unix

Major mode: Dired by name

Minor modes in effect:
  dired-omit-mode: t
  save-place-mode: t
  savehist-mode: t
  tooltip-mode: t
  mouse-wheel-mode: t
  file-name-shadow-mode: t
  global-font-lock-mode: t
  font-lock-mode: t
  blink-cursor-mode: t
  auto-composition-mode: t
  auto-encryption-mode: t
  auto-compression-mode: t
  buffer-read-only: t
  column-number-mode: 1
  line-number-mode: t
  transient-mark-mode: t

Load-path shadows:
~/dev/elisp/dictionary-el/dictionary hides /usr/local/share/emacs/28.0.50/lisp/net/dictionary

Features:
(shadow face-remap mu4e mu4e-org mu4e-main mu4e-view mu4e-view-old
mu4e-view-common thingatpt mu4e-headers mu4e-compose mu4e-context
mu4e-draft mu4e-actions rfc2368 smtpmail mu4e-mark mu4e-proc mu4e-utils
doc-view jka-compr image-mode exif mu4e-lists mu4e-message shr kinsoku
svg xml dom browse-url url url-proxy url-privacy url-expand url-methods
url-history url-cookie url-domsuf url-util url-parse url-vars flow-fill
org ob ob-tangle ob-ref ob-lob ob-table ob-exp org-macro org-footnote
org-src ob-comint org-pcomplete pcomplete comint ansi-color org-list
org-faces org-entities noutline outline org-version ob-emacs-lisp
ob-core ob-eval org-table ol org-keys org-compat advice org-macs
org-loaddefs format-spec cal-menu calendar cal-loaddefs mule-util
mailcap hl-line mu4e-vars mu4e-meta add-log log-view pcvs-util vc-mtn
vc-hg vc-git diff-mode easy-mmode vc-bzr vc-src vc-sccs vc-svn vc-cvs
vc-rcs vc vc-dispatcher dired-aux nswbuff eieio-opt cl-extra speedbar
ezimage dframe find-func shortdoc help-fns radix-tree help-mode emacsbug
message rmc puny rfc822 mml mml-sec epa derived epg epg-config gnus-util
rmail rmail-loaddefs auth-source cl-seq eieio eieio-core cl-macs
eieio-loaddefs password-cache json map text-property-search time-date
subr-x mm-decode mm-bodies mm-encode mail-parse rfc2231 mailabbrev
gmm-utils mailheader sendmail rfc2047 rfc2045 ietf-drums mm-util
mail-prsvr mail-utils edmacro kmacro cl-loaddefs cl-lib xcscope ring ido
seq byte-opt gv bytecomp byte-compile cconv dired-x dired dired-loaddefs
saveplace savehist iso-transl tooltip eldoc electric uniquify ediff-hook
vc-hooks lisp-float-type mwheel term/x-win x-win term/common-win x-dnd
tool-bar dnd fontset image regexp-opt fringe tabulated-list replace
newcomment text-mode elisp-mode lisp-mode prog-mode register page
tab-bar menu-bar rfn-eshadow isearch easymenu timer select scroll-bar
mouse jit-lock font-lock syntax font-core term/tty-colors frame
minibuffer cl-generic cham georgian utf-8-lang misc-lang vietnamese
tibetan thai tai-viet lao korean japanese eucjp-ms cp51932 hebrew greek
romanian slovak czech european ethiopic indian cyrillic chinese
composite charscript charprop case-table epa-hook jka-cmpr-hook help
simple abbrev obarray cl-preloaded nadvice button loaddefs faces
cus-face macroexp files window text-properties overlay sha1 md5 base64
format env code-pages mule custom widget hashtable-print-readable
backquote threads inotify dynamic-setting font-render-setting x-toolkit
x multi-tty make-network-process emacs)

Memory information:
((conses 16 151859 9119)
 (symbols 48 16246 5)
 (strings 32 57927 2771)
 (string-bytes 1 1891100)
 (vectors 16 31514)
 (vector-slots 8 344237 13676)
 (floats 8 170 151)
 (intervals 56 1810 15)
 (buffers 992 14))

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

[martin rudalics]

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

 > quit-window doesn't work properly with the master branch.
 >
 > To see the issue with emacs -Q, in any git repo:
 >
 > * C-x v L
 > * C-x 1 (if the screen is split).
 > * Hitting 'q' now doesn't close the window.
 >
 > Looks like commit 0a681590268a4039f95a5a919b6b6d4f4e880d4c is
 > the problem.

Right.  Can you try the attached patch?  pillule, please also have a
look into this.

Thanks for the report, martin

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: quit-restore-window.diff --]
[-- Type: text/x-patch; name="quit-restore-window.diff", Size: 603 bytes --]

diff --git a/lisp/window.el b/lisp/window.el
index 026cde5901..8a767140bc 100644
--- a/lisp/window.el
+++ b/lisp/window.el
@@ -5110,8 +5110,7 @@ quit-restore-window
       (set-window-parameter window 'quit-restore nil)
       ;; Make sure that WINDOW is no more dedicated.
       (set-window-dedicated-p window nil)
-      (if prev-buffer
-          (switch-to-prev-buffer window bury-or-kill)
+      (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#48493: 28.0.50; quit-window doesn't work

[Sujith Manoharan]

martin rudalics <rudalics@gmx.at> writes:
> Right.  Can you try the attached patch?  pillule, please also have a
> look into this.

The patch fixes the issue. Thanks.

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

[martin rudalics]

 > The patch fixes the issue. Thanks.

Thanks for checking.  Unless pillule has a better idea I'll install
that.

martin

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

[martin rudalics]

;; tags 48493 fixed
;; close 48493 28.1
;; quit

 > The patch fixes the issue. Thanks.

Installed and bug marked as done.

Thanks again for the report, martin

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

[pillule]

martin rudalics <rudalics@gmx.at> writes:

>  > Looks like commit 0a681590268a4039f95a5a919b6b6d4f4e880d4c is
>  > the problem.

Sorry about that.

> Right.  Can you try the attached patch?  pillule, please also 
> have a
> look into this.

I think there is still edges cases :

with emacs -Q

;; set some rules
(setq display-buffer-alist
     '(("\\*\\(Backtrace\\)\\*"
       (display-buffer-in-side-window)
       (window-height . 0.2)
       (side . bottom))
       ("\\*\\(Messages\\)\\*"
       (display-buffer-in-side-window)
       (window-height . 0.2)
       (side . bottom))))

;; display the *Messages* buffer
(view-echo-area-messages)

;; trigger a logical error to display the *Backtrace* buffer
(> vim emacs)

;; trigger a quit-restore-window by calling kill-buffer in a 
   dedicated window
(kill-buffer (get-buffer "*Backtrace*"))
;; the dedicated property has been cleaned from the window,
;; yet *Messages* is displayed in place of this window !
(kill-buffer (get-buffer "*Messages*"))
;; calling kill-buffer in a non-dedicated window,
;; does not trigger quit-restore-window anymore
;; resulting in an undesirable buffer being displayed in place.


What is the desired behavior of a dedicated window here ?
Must *Messages* be displayed after we killed *Backtrace* in the 
first place ?
I am suspecting that the locale value of `kill-buffer-hook' in 
*Backtrace* is interfering with this test but I have difficulties 
to find its human readable form.

-- 

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

[martin rudalics]

 > I think there is still edges cases :
 >
 > with emacs -Q
 >
 > ;; set some rules
 > (setq display-buffer-alist
 >      '(("\\*\\(Backtrace\\)\\*"
 >        (display-buffer-in-side-window)
 >        (window-height . 0.2)
 >        (side . bottom))
 >        ("\\*\\(Messages\\)\\*"
 >        (display-buffer-in-side-window)
 >        (window-height . 0.2)
 >        (side . bottom))))
 >
 > ;; display the *Messages* buffer
 > (view-echo-area-messages)
 >
 > ;; trigger a logical error to display the *Backtrace* buffer
 > (> vim emacs)
 >
 > ;; trigger a quit-restore-window by calling kill-buffer in a   dedicated window
 > (kill-buffer (get-buffer "*Backtrace*"))
 > ;; the dedicated property has been cleaned from the window,
 > ;; yet *Messages* is displayed in place of this window !
 > (kill-buffer (get-buffer "*Messages*"))
 > ;; calling kill-buffer in a non-dedicated window,
 > ;; does not trigger quit-restore-window anymore
 > ;; resulting in an undesirable buffer being displayed in place.
 >
 >
 > What is the desired behavior of a dedicated window here ?
 > Must *Messages* be displayed after we killed *Backtrace* in the first
 > place ?

I think so, yes.

 > I am suspecting that the locale value of `kill-buffer-hook' in
 > *Backtrace* is interfering with this test but I have difficulties to
 > find its human readable form.

I doubt that.  It's probably due to my fix for Bug#48493.  When
*Messages* is killed, the window should have no more previous buffers to
display and we should be able to kill it.  But `switch-to-prev-buffer'
shows the "next" buffer instead.  Try to come up with a solution that
does not reintroduce Bug#48493 and deletes the window when no previous
buffer is found.

martin

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

[pillule]

>> What is the desired behavior of a dedicated window here ?
>> Must *Messages* be displayed after we killed *Backtrace* in the 
>> first
>> place ?
>
> I think so, yes.

Then I suppose that the dedicated window parameter must be 
restored
after a kill-buffer accordingly; this solve the previous test but
ask for more modifications.

I am testing a version of this.

> [...] When
> *Messages* is killed, the window should have no more previous 
> buffers to
> display and we should be able to kill it.  But 
> `switch-to-prev-buffer'
> shows the "next" buffer instead.  Try to come up with a solution 
> that
> does not reintroduce Bug#48493 and deletes the window when no 
> previous
> buffer is found.

I can modify to `switch-to-prev-buffer' (and its sibling
`switch-to-prev-buffer') to return nil instead of the current 
buffer;
however the result is the same : the window rest in place with an
undesired buffer inside.
Note that we may want that anyway if it can solve the cases where
`quit-restore-window' display the same buffer again.

I am still looking to find what may be messing the prev-buffers 
list.

--

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

[martin rudalics]

 > Then I suppose that the dedicated window parameter must be restored
 > after a kill-buffer accordingly; this solve the previous test but
 > ask for more modifications.

Who makes any of these buffers dedicared?

 > I can modify to `switch-to-prev-buffer' (and its sibling
 > `switch-to-prev-buffer') to return nil instead of the current buffer;
 > however the result is the same : the window rest in place with an
 > undesired buffer inside.
 > Note that we may want that anyway if it can solve the cases where
 > `quit-restore-window' display the same buffer again.
 >
 > I am still looking to find what may be messing the prev-buffers list.

Try to experiment with an idiom like

       (if prev-buffer
           ;; If a previous buffer exists, try to switch to it.  If that
           ;; fails for whatever reason, try to delete the window.
           (unless (switch-to-prev-buffer window bury-or-kill)
             (window--delete window nil (eq bury-or-kill 'kill)))
         ;; If no previous buffer exists, try to delete the window.  If
         ;; that fails for whatever reason, try to switch to some other
         ;; buffer.
         (unless (window--delete window nil (eq bury-or-kill 'kill))
           (switch-to-prev-buffer window bury-or-kill)))

martin

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

[pillule]

martin rudalics <rudalics@gmx.at> writes:

>> Then I suppose that the dedicated window parameter must be 
>> restored
>> after a kill-buffer accordingly; this solve the previous test 
>> but
>> ask for more modifications.
>
> Who makes any of these buffers dedicared?

All windows created by `display-buffer-in-side-window' have the 
state
(dedicated . side)

Tiers libraries such as the module popup from Doom emacs come at 
this
subject with the rule :

--quitting/killing a buffer in a side window,
always delete its window--
(it is done with a local value of kill-buffer-hook installed from
their own `display-buffer-alist')

instead of the rule :

--quitting/killing a buffer in any window,
1 switch to its previous buffer
2 delete the window if no available
3 switch to another buffer if the window is not deletable--

Which is what I think you are asking and IMHO ask to deal with the
dedicated window state to not cripple side-windows.

>> I can modify to `switch-to-prev-buffer' (and its sibling
>> `switch-to-prev-buffer') to return nil instead of the current 
>> buffer;
>> however the result is the same : the window rest in place with 
>> an
>> undesired buffer inside.
>> Note that we may want that anyway if it can solve the cases 
>> where
>> `quit-restore-window' display the same buffer again.
>>
>> I am still looking to find what may be messing the prev-buffers 
>> list.
>
> Try to experiment with an idiom like
>
>       (if prev-buffer
>           ;; If a previous buffer exists, try to switch to it. 
>           If that
>           ;; fails for whatever reason, try to delete the 
>           window.
>           (unless (switch-to-prev-buffer window bury-or-kill)
>             (window--delete window nil (eq bury-or-kill 'kill)))
>         ;; If no previous buffer exists, try to delete the 
>         window.  If
>         ;; that fails for whatever reason, try to switch to some 
>         other
>         ;; buffer.
>         (unless (window--delete window nil (eq bury-or-kill 
>         'kill))
>           (switch-to-prev-buffer window bury-or-kill)))

Thanks for the snippet, I think I am confused by this window 
dedication,
please help me to clarify my mind before I try to implement a 
solution
with or without the dedicated window state.

-- 

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

[martin rudalics]

 > Which is what I think you are asking and IMHO ask to deal with the
 > dedicated window state to not cripple side-windows.

You're right, I forgot that side windows are by default dedicated to
their buffers.

The dedicated flag in a side window serves to prevent "normal" buffer
display to "avoid" that window.  A side window may be reused for showing
another buffer only by `display-buffer-in-side-window'.  This sense of
dedication is somewhat different from the normal sense where killing the
buffer always attempts to delete its dedicated windows (and maybe their
frames too) first.

Hence it is perfectly valid to switch to the previous buffer here - any
such buffer was meant to be displayed in that side window.  It would be,
however, invalid to switch to some buffer that was never shown in that
window here.  Note in this context that we can always delete a side
window, a side window can never be alone on its frame.

 > Thanks for the snippet, I think I am confused by this window dedication,
 > please help me to clarify my mind before I try to implement a solution
 > with or without the dedicated window state.

So maybe something like this might cut it:

       (if (and prev-buffer (memq (window-dedicated-p window) '(nil side)))
           ;; If a previous buffer exists, try to switch to it.  If that
           ;; fails for whatever reason, try to delete the window.
           (unless (switch-to-prev-buffer window bury-or-kill)
             (window--delete window nil (eq bury-or-kill 'kill)))
         ;; If no previous buffer exists, try to delete the window.  If
         ;; that fails for whatever reason, try to switch to some other
         ;; buffer.
         (unless (window--delete window nil (eq bury-or-kill 'kill))
           (switch-to-prev-buffer window bury-or-kill)))

Tell me whether this works with DOOM's `kill-buffer-hook'.

If you feel that it's more natural to delete the window in the case at
hand, we can consider that too.  But suppose someone uses such a side
window for something more permanent like a compile or shell buffer and
the backtrace buffer kicked in only accidentally, then deleting the side
window when killing the backtrace buffer might not be a good idea.

martin

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

[pillule]

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


martin rudalics <rudalics@gmx.at> writes:

> The dedicated flag in a side window serves to prevent "normal" buffer
> display to "avoid" that window.  A side window may be reused for showing
> another buffer only by `display-buffer-in-side-window'.  This sense of
> dedication is somewhat different from the normal sense where killing the
> buffer always attempts to delete its dedicated windows (and maybe their
> frames too) first.
>
> Hence it is perfectly valid to switch to the previous buffer here - any
> such buffer was meant to be displayed in that side window.  It would be,
> however, invalid to switch to some buffer that was never shown in that
> window here.  Note in this context that we can always delete a side
> window, a side window can never be alone on its frame.

Yes.

> Tell me whether this works with DOOM's `kill-buffer-hook'.

I can test the changes against a version of DOOM, yes. For the draft below
it seems to be ok, but keep in mind that their library bypass these parts
window.el

> If you feel that it's more natural to delete the window in the case at
> hand, we can consider that too.

Not at all. For me it is ok with switch-to-prev-buffer, if users want to delete
the window and/or buffer explicitly, they have commands for that. In the case of
DOOM it is implemented as a workaround against some bugs, it is
explicated in :

(+popup/quit-window)
Documentation
The regular quit-window sometimes kills the popup buffer and switches to a
buffer that shouldn't be in a popup. We prevent that by remapping quit-window
to this commmand.


So here is the *draft* that pass the previous cases of this thread.
`replace-buffer-in-windows' take care of killing buffers.
I restore the dedication of side-window because :
1. it seems to me it is the right think to do, and it prevents 2.
2. I lost the trace when we kill a buffer and no previous-buffer is found
but  still an undesirable buffer replace the current,
is it in the C part ? I can't inspect C...


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: better handling of side windows --]
[-- Type: text/x-diff, Size: 5414 bytes --]

From 8305210da9e2215a8b38cd5cf88cd4c6c856fa0b Mon Sep 17 00:00:00 2001
From: Trust me I am a doctor <pillule@riseup.net>
Date: Wed, 26 May 2021 15:40:39 +0200
Subject: [PATCH] Better handling of side-windows

* lisp/window.el
   (switch-to-prev-buffer) : Return nil when prev-buffer unavailable
   (switch-to-next-buffer) : Return nil when next-buffer unavailable
   (replace-buffer-in-windows) : Does not systematically delete
side-windows, instead try to `switch-to-prev-buffer' and delete the
window only if prev-buffer was unavailable
   (quit-restore-window) : Add exceptions for side-windows, so it
try to to `switch-to-prev-buffer' and delete the window only if
prev-buffer was unavailable (bug#48493)
---
 lisp/window.el | 38 +++++++++++++++++++++++++++-----------
 1 file changed, 27 insertions(+), 11 deletions(-)

diff --git a/lisp/window.el b/lisp/window.el
index fd1c617d6b..33f4409fcf 100644
--- a/lisp/window.el
+++ b/lisp/window.el
@@ -4559,11 +4559,11 @@ This function is called by `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 +4726,11 @@ This function is called by `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 (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)))
           (if (switch-to-prev-buffer-skip-p skip window new-buffer)
 	      (setq skipped (or skipped new-buffer))
@@ -4989,10 +4989,14 @@ all window-local buffer lists."
   (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))
+	  (let ((dedicated-side (eq (window-dedicated-p window) 'side)))
+            ;; Try to delete the window, with the exception of side-windows
+            (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)))))
 
@@ -5040,6 +5044,7 @@ nil means to not handle the buffer in a particular way.  This
                         (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)
@@ -5084,6 +5089,8 @@ nil means to not handle the buffer in a particular way.  This
       ;; 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))
+      ;; Preserve the side-window dedication
+      (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 +5117,18 @@ nil means to not handle the buffer in a particular way.  This
       (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)))))
+      (if (and prev-buffer (eq dedicated 'side))
+          ;; If a previous buffer exists, try to switch to it and
+          ;; to preserve the side-window dedication. If that
+          ;; fails for whatever reason, try to delete the window.
+          (if (switch-to-prev-buffer window bury-or-kill)
+              (set-dedicated-window-p window 'side)
+            (window--delete window nil (eq bury-or-kill 'kill)))
+          ;; If no previous buffer exists, try to delete the window.  If
+          ;; that fails for whatever reason, try to switch to some other
+          ;; buffer.
+          (unless (window--delete window nil (eq bury-or-kill 'kill))
+            (switch-to-prev-buffer window bury-or-kill)))))
 
     ;; Deal with the buffer.
     (cond
-- 
2.20.1


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


I will pass the rest of they day to look at ERT to see if I can learn
how to write reusable tests and see if this discussion needs to be reflected
in the documentation.

--

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

[martin rudalics]

 > I can test the changes against a version of DOOM, yes. For the draft below
 > it seems to be ok, but keep in mind that their library bypass these parts
 > window.el

You already told me so.

 >> If you feel that it's more natural to delete the window in the case at
 >> hand, we can consider that too.
 >
 > Not at all. For me it is ok with switch-to-prev-buffer, if users want to delete
 > the window and/or buffer explicitly, they have commands for that. In the case of
 > DOOM it is implemented as a workaround against some bugs, it is
 > explicated in :
 >
 > (+popup/quit-window)
 > Documentation
 > The regular quit-window sometimes kills the popup buffer and switches to a
 > buffer that shouldn't be in a popup. We prevent that by remapping quit-window
 > to this commmand.

Maybe this is precisely the behavior you're trying to fix and their fix
won't then be needed any more.

 > So here is the *draft* that pass the previous cases of this thread.
 > `replace-buffer-in-windows' take care of killing buffers.
 > I restore the dedication of side-window because :
 > 1. it seems to me it is the right think to do, and it prevents 2.

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?

 > 2. I lost the trace when we kill a buffer and no previous-buffer is found
 > but  still an undesirable buffer replace the current,
 > is it in the C part ? I can't inspect C...

IIUC you mean the one in `kill-buffer' in buffer.c.  `kill-buffer' first
calls

   replace_buffer_in_windows (buffer);

which simply calls the Lisp function `replace-buffer-in-windows' for
buffer (the buffer to kill) and later on does

   replace_buffer_in_windows_safely (buffer);

That last call will show another buffer in a window if and only if
`replace-buffer-in-windows' failed to do its work which is an unusual
case - one that should _not_ have happened.  But we must, in C, simply
make sure that such a failure gets caught since showing a dead buffer in
a live window can crash Emacs.  So if replace_buffer_in_windows_safely
shows another buffer, then something in `replace-buffer-in-windows' was
already broken before and we need not bother to clean up things.

 > I will pass the rest of they day to look at ERT to see if I can learn
 > how to write reusable tests and see if this discussion needs to be reflected
 > in the documentation.

We have to document it in the Elisp manual.

martin

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

[pillule]

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


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.


[-- Attachment #2: Improve handling of side dedicated flag --]
[-- Type: text/x-diff, Size: 21543 bytes --]

From 4327b124afd2169e218509143175d1a20f9f65ce Mon Sep 17 00:00:00 2001
From: Trust me I am a doctor <pillule@riseup.net>
Date: Tue, 8 Jun 2021 01:12:03 +0200
Subject: [PATCH] Improve handling of dedicated side flag when quitting window

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, try to
`switch-to-prev-buffer' before deleting a side window, fix (Bug#48367)
---
 doc/lispref/windows.texi |  58 +++++++----
 lisp/window.el           | 216 ++++++++++++++++++++++-----------------
 2 files changed, 159 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..f619372af8 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,101 @@ 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)
+        (if (and prev-buffer (eq dedicated 'side)) ;;  (Bug#48367)
+            ;; If a previous buffer exists, try to switch to it.  If that
+            ;; fails for whatever reason, try to delete the window.
+            (if (switch-to-prev-buffer window bury-or-kill)
+                (set-window-dedicated-p window 'side)
+              (window--delete window nil (eq bury-or-kill 'kill)))
+          ;; If no previous buffer exists, try to delete the window.  If
+          ;; that fails for whatever reason, try to switch to some other
+          ;; buffer.
+          (unless (window--delete window nil (eq bury-or-kill 'kill))
+            (switch-to-prev-buffer window bury-or-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: 6 bytes --]




--

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

[pillule]

[-- 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 --]



--

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

[Eli Zaretskii]

> From: pillule <pillule@riseup.net>
> Date: Tue, 08 Jun 2021 01:23:11 +0200
> Cc: pillule <pillule@riseup.net>, Sujith Manoharan <sujith.wall@gmail.com>,
>  48493@debbugs.gnu.org
> 
> > 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.

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

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

[martin rudalics]

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

+    (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))

BTW, how's your paperwork process proceeding?

martin

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

[pillule]

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


Eli Zaretskii <eliz@gnu.org> writes:

>> From: pillule <pillule@riseup.net>
>> Date: Tue, 08 Jun 2021 01:23:11 +0200
>> Cc: pillule <pillule@riseup.net>, Sujith Manoharan 
>> <sujith.wall@gmail.com>,
>>  48493@debbugs.gnu.org
>>
>> > 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 <rudalics@gmx.at> 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.


[-- Attachment #2: respect the indentation --]
[-- Type: text/x-diff, Size: 14704 bytes --]

From 39fc2754de267cba77dcb9c8c567fc82bdcd17f9 Mon Sep 17 00:00:00 2001
From: Trust me I am a doctor <pillule@riseup.net>
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


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


--

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

[pillule]

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

I read it again and think you were right,
when  (window--delete window 'dedicated (eq bury-or-kill 'kill))
is part of the conditional, it indeed already fail if the window 
is not deletable ;

I will correct that in the next revision.

--

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

[pillule]

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


pillule <pillule@riseup.net> writes:

>>> 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
>>
>
> I read it again and think you were right,
> when  (window--delete window 'dedicated (eq bury-or-kill 'kill))
> is part of the conditional, it indeed already fail if the window 
> is not deletable ;
>
> I will correct that in the next revision.

hm. here again minors corrections. sorry for the noise.


[-- Attachment #2: good ol'cond --]
[-- Type: text/x-diff, Size: 14697 bytes --]

From a0c037f8d1896f1e85570df04b1358504ea547b6 Mon Sep 17 00:00:00 2001
From: Trust me I am a doctor <pillule@riseup.net>
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           | 63 ++++++++++++++++++++++++++++------------
 2 files changed, 80 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..0504abb8c6 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,12 @@ 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--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 +5102,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 +5130,14 @@ 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)
+          (when (eq dedicated 'side)
+            (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


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


--

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

[martin rudalics]

 > hm. here again minors corrections. sorry for the noise.

OK.  Please inform me as soon as your paperwork is complete.

Thanks, martin

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

[pillule]

martin rudalics <rudalics@gmx.at> writes:

>> hm. here again minors corrections. sorry for the noise.
>
> OK.  Please inform me as soon as your paperwork is complete.
>
> Thanks, martin

I get the papers, get the papers !

--

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

[martin rudalics]

> I get the papers, get the papers !

Congratulations!

martin

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

[martin rudalics]

 > here again minors corrections. sorry for the noise.

I pushed that change now with some minor modifications.  Please have a
look.

Thanks, martin

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

[pillule]

martin rudalics <rudalics@gmx.at> writes:

>> here again minors corrections. sorry for the noise.
>
> I pushed that change now with some minor modifications.  Please have a
> look.
>
> Thanks, martin

Thanks :)

--