all messages for Emacs-related lists mirrored at yhetil.org
 help / color / mirror / code / Atom feed
* bug#59862: quit-restore per window buffer
@ 2022-12-06 17:32 Juri Linkov
  2024-06-02  6:45 ` Juri Linkov
  0 siblings, 1 reply; 15+ messages in thread
From: Juri Linkov @ 2022-12-06 17:32 UTC (permalink / raw)
  To: 59862

1. C-x 4 d RET C-h C-t q q         -- the bottom window is NOT deleted
2. C-x 4 d RET C-h C-t C-x k RET q -- the bottom window is deleted
3. C-x 4 d RET C-h C-n C-x k RET q -- the bottom window is NOT deleted

The accidental difference between the last two is that the former uses
`switch-to-buffer' whereas the latter uses `pop-to-buffer-same-window'.

The root of the problem is that displaying a buffer in an existing
window, or quitting a buffer in an existing window overwrites its
window parameter `quit-restore', and thus invalidates the history of
how the first window was displayed.

A partial fix that solves only the first test case above is at least
not to overwrite `quit-restore' on quitting other buffers in the same
window:

```
diff --git a/lisp/window.el b/lisp/window.el
index a11293d372a..e3b057599d5 100644
--- a/lisp/window.el
+++ b/lisp/window.el
@@ -5275,14 +5276,14 @@ quit-restore-window
 	(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.
       ;; If the previously selected window is still alive, select it.
       (window--quit-restore-select-window quit-restore-2))
      (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)
       ;; Try to switch to a previous buffer.  Delete the window only if
```

But the proper fix for other problems would be to replace the window
parameter `quit-restore' currently shared by all buffers in the same window
by a stack where every window buffer should keep own quit-restore data.
There is already such a stack in `window-prev-buffers', so a possible
solution would be to add quit-restore data to each buffer
in window-prev-buffers.

Alternatively, it would be nice to have an option that would prevent
overwriting the initial value of the window parameter `quit-restore',
thus `quit-restore-window' could delete the window regardless of
how many buffers were displayed in that window, like it does in case
of dedicated windows.





^ permalink raw reply related	[flat|nested] 15+ messages in thread

* bug#59862: quit-restore per window buffer
  2022-12-06 17:32 bug#59862: quit-restore per window buffer Juri Linkov
@ 2024-06-02  6:45 ` Juri Linkov
  2024-06-03  9:34   ` martin rudalics via Bug reports for GNU Emacs, the Swiss army knife of text editors
  0 siblings, 1 reply; 15+ messages in thread
From: Juri Linkov @ 2024-06-02  6:45 UTC (permalink / raw)
  To: 59862; +Cc: martin rudalics

Martin, do you agree with this?  I tested this patch for a long time.

> 1. C-x 4 d RET C-h C-t q q         -- the bottom window is NOT deleted
> 2. C-x 4 d RET C-h C-t C-x k RET q -- the bottom window is deleted
> 3. C-x 4 d RET C-h C-n C-x k RET q -- the bottom window is NOT deleted
>
> The accidental difference between the last two is that the former uses
> `switch-to-buffer' whereas the latter uses `pop-to-buffer-same-window'.
>
> The root of the problem is that displaying a buffer in an existing
> window, or quitting a buffer in an existing window overwrites its
> window parameter `quit-restore', and thus invalidates the history of
> how the first window was displayed.
>
> A partial fix that solves only the first test case above is at least
> not to overwrite `quit-restore' on quitting other buffers in the same
> window:
>
> diff --git a/lisp/window.el b/lisp/window.el
> index a11293d372a..e3b057599d5 100644
> --- a/lisp/window.el
> +++ b/lisp/window.el
> @@ -5275,14 +5276,14 @@ quit-restore-window
>  	(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.
>        ;; If the previously selected window is still alive, select it.
>        (window--quit-restore-select-window quit-restore-2))
>       (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)
>        ;; Try to switch to a previous buffer.  Delete the window only if
>
> But the proper fix for other problems would be to replace the window
> parameter `quit-restore' currently shared by all buffers in the same window
> by a stack where every window buffer should keep own quit-restore data.
> There is already such a stack in `window-prev-buffers', so a possible
> solution would be to add quit-restore data to each buffer
> in window-prev-buffers.
>
> Alternatively, it would be nice to have an option that would prevent
> overwriting the initial value of the window parameter `quit-restore',
> thus `quit-restore-window' could delete the window regardless of
> how many buffers were displayed in that window, like it does in case
> of dedicated windows.





^ permalink raw reply	[flat|nested] 15+ messages in thread

* bug#59862: quit-restore per window buffer
  2024-06-02  6:45 ` Juri Linkov
@ 2024-06-03  9:34   ` martin rudalics via Bug reports for GNU Emacs, the Swiss army knife of text editors
  2024-06-03  9:53     ` martin rudalics via Bug reports for GNU Emacs, the Swiss army knife of text editors
  0 siblings, 1 reply; 15+ messages in thread
From: martin rudalics via Bug reports for GNU Emacs, the Swiss army knife of text editors @ 2024-06-03  9:34 UTC (permalink / raw)
  To: Juri Linkov, 59862

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

 > Martin, do you agree with this?  I tested this patch for a long time.
 >
 >> 1. C-x 4 d RET C-h C-t q q         -- the bottom window is NOT deleted
 >> 2. C-x 4 d RET C-h C-t C-x k RET q -- the bottom window is deleted
 >> 3. C-x 4 d RET C-h C-n C-x k RET q -- the bottom window is NOT deleted
 >>
 >> The accidental difference between the last two is that the former uses
 >> `switch-to-buffer' whereas the latter uses `pop-to-buffer-same-window'.

'switch-to-buffer' leaves the 'quit-restore' parameter the way it was
set up by C-x 4 d.  'pop-to-buffer-same-window' changes it to the
'reuse' type.  So one fix is to have 'switch-to-buffer' record the
window when it does not obey display actions and it gets the 'reuse'
type as well.  This way there are no more accidental differences when
quitting the help buffer.

 >> The root of the problem is that displaying a buffer in an existing
 >> window, or quitting a buffer in an existing window overwrites its
 >> window parameter `quit-restore', and thus invalidates the history of
 >> how the first window was displayed.

We could fix the former in 'display-buffer-record-window' by having it
not overwrite an existing 'quit-restore' parameter when it displays
another buffer.  Then 'quit-window' would always delete the window in
the scenarios above instead of showing the dir buffer.  The backside is
that additional information recorded by 'display-buffer-record-window'
like the now selected window or window sizes would no more get recorded.

 >> A partial fix that solves only the first test case above is at least
 >> not to overwrite `quit-restore' on quitting other buffers in the same
 >> window:

You cannot simply fix the third test in 'quit-restore-window' alone
because C-x k leaves the 'quit-restore' parameter of the NEWS buffer
unchanged.  That's arguably a bug: When killing buffers and in some
other cases we should remove the 'quit-restore' parameter.

 >> diff --git a/lisp/window.el b/lisp/window.el
 >> index a11293d372a..e3b057599d5 100644
 >> --- a/lisp/window.el
 >> +++ b/lisp/window.el
 >> @@ -5275,14 +5276,14 @@ quit-restore-window
 >>   	(set-window-prev-buffers
 >>   	 window (append (window-prev-buffers window) (list entry))))
 >>         ;; Reset the quit-restore parameter.
 >> -      (set-window-parameter window 'quit-restore nil)

Why keep it here?  After all, this one has accomplished its mission so
keeping it looks like an UXB to me.

 >>         ;; Select old window.
 >>         ;; If the previously selected window is still alive, select it.
 >>         (window--quit-restore-select-window quit-restore-2))
 >>        (t
 >>         ;; Show some other buffer in WINDOW and reset the quit-restore
 >>         ;; parameter.
 >> -      (set-window-parameter window 'quit-restore nil)

Keeping this one looks OK.  If the buffer shown in the window is not the
one the parameter mentions, chances are that a 'quit-restore-window' can
eventually use it in the future.

 >> But the proper fix for other problems would be to replace the window
 >> parameter `quit-restore' currently shared by all buffers in the same window
 >> by a stack where every window buffer should keep own quit-restore data.
 >> There is already such a stack in `window-prev-buffers', so a possible
 >> solution would be to add quit-restore data to each buffer
 >> in window-prev-buffers.

And how would such a stack handle intertwined 'next-buffer' and
'prev-buffer' calls?  Stacks are one of those things that set apart
computers from human beings.

 >> Alternatively, it would be nice to have an option that would prevent
 >> overwriting the initial value of the window parameter `quit-restore',
 >> thus `quit-restore-window' could delete the window regardless of
 >> how many buffers were displayed in that window, like it does in case
 >> of dedicated windows.

I attach a patch that tries to do that.  Here it handles all scenarios
you describe above.  However, it's by no means trivial and requires at
least _one month_ of testing on your side.  I have not addressed the

  	   (eq (nth 1 quit-restore) 'tab)

case in 'quit-restore-window'.  You have to look into this yourself.
And there might obviously be other cases missing.

martin

[-- Attachment #2: quit-restore.diff --]
[-- Type: text/x-patch, Size: 9910 bytes --]

diff --git a/lisp/window.el b/lisp/window.el
index b014be6a7cf..8b79a89270b 100644
--- a/lisp/window.el
+++ b/lisp/window.el
@@ -5108,8 +5108,8 @@ delete-windows-on
   (let ((buffer (window-normalize-buffer buffer-or-name))
 	;; Handle the "inverted" meaning of the FRAME argument wrt other
 	;; `window-list-1' based function.
-	(all-frames (cond ((not frame) t) ((eq frame t) nil) (t frame))))
-    (dolist (window (window-list-1 nil nil all-frames))
+	(frames (cond ((not frame) t) ((eq frame t) nil) (t frame))))
+    (dolist (window (window-list-1 nil 'nomini frames))
       (if (eq (window-buffer window) buffer)
 	  (let ((deletable (window-deletable-p window))
                 (dedicated (window-dedicated-p window)))
@@ -5121,12 +5121,18 @@ delete-windows-on
 	      ;; Delete window.
 	      (delete-window window))
 	     (t
-	      ;; In window switch to previous buffer.
-	      (set-window-dedicated-p window nil)
-	      (switch-to-prev-buffer window 'bury)
-              ;; Restore the dedicated 'side' flag.
-              (when (eq dedicated 'side)
-                (set-window-dedicated-p window 'side)))))
+	      (when (eq (window-buffer window) buffer)
+		(quit-restore-window window))
+	      ;; Remove quit-restore(-prev) entries that mention BUFFER.
+	      (let ((quit-restore (window-parameter window 'quit-restore))
+		    (quit-restore-prev (window-parameter window 'quit-restore-prev)))
+		(when (eq buffer (nth 3 quit-restore-prev))
+		  (set-window-parameter window 'quit-restore-prev nil))
+		(when (eq buffer (nth 3 quit-restore))
+		  (set-window-parameter
+		   window 'quit-restore (window-parameter window 'quit-restore-prev))))
+	      ;; Unrecord BUFFER in this window.
+	      (unrecord-window-buffer window buffer))))
 	;; If a window doesn't show BUFFER, unrecord BUFFER in it.
 	(unrecord-window-buffer window buffer)))))
 
@@ -5146,17 +5152,18 @@ replace-buffer-in-windows
   (interactive "bBuffer to replace: ")
   (let ((buffer (window-normalize-buffer buffer-or-name)))
     (dolist (window (window-list-1 nil nil t))
-      (if (eq (window-buffer window) buffer)
-          ;; Delete a dedicated window unless it is a side window.
-          (let ((dedicated-side (eq (window-dedicated-p window) 'side)))
-            (when (or dedicated-side (not (window--delete window t t)))
-              ;; Switch to another buffer in that 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)))))
+      (when (eq (window-buffer window) buffer)
+	(quit-restore-window window 'kill))
+      ;; Remove quit-restore(-prev) entries that mention BUFFER.
+      (let ((quit-restore (window-parameter window 'quit-restore))
+	    (quit-restore-prev (window-parameter window 'quit-restore-prev)))
+	(when (eq buffer (nth 3 quit-restore-prev))
+	  (set-window-parameter window 'quit-restore-prev nil))
+	(when (eq buffer (nth 3 quit-restore))
+	  (set-window-parameter
+	   window 'quit-restore (window-parameter window 'quit-restore-prev))))
+      ;; Unrecord BUFFER in this window.
+      (unrecord-window-buffer window buffer))))
 
 (defcustom quit-window-hook nil
   "Hook run before performing any other actions in the `quit-window' command."
@@ -5210,13 +5217,15 @@ quit-restore-window
   (setq window (window-normalize-window window t))
   (let* ((buffer (window-buffer window))
 	 (quit-restore (window-parameter window 'quit-restore))
+	 (quit-restore-prev (window-parameter window 'quit-restore-prev))
 	 (quit-restore-2 (nth 2 quit-restore))
+	 (quit-restore-prev-2 (nth 2 quit-restore-prev))
          (prev-buffer (catch 'prev-buffer
                         (dolist (buf (window-prev-buffers window))
                           (unless (eq (car buf) buffer)
                             (throw 'prev-buffer (car buf))))))
          (dedicated (window-dedicated-p window))
-	 quad entry)
+	 quad entry reset-prev)
     (cond
      ;; First try to delete dedicated windows that are not side windows.
      ((and dedicated (not (eq dedicated 'side))
@@ -5230,21 +5239,35 @@ quit-restore-window
       ;; If the previously selected window is still alive, select it.
       (window--quit-restore-select-window quit-restore-2))
      ((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)
+	   (or (and (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))
+	       (and (or (eq (nth 1 quit-restore-prev) 'frame)
+			(and (eq (nth 1 quit-restore-prev) 'window)
+			     (not (eq window (frame-root-window window)))))
+		    (eq (nth 3 quit-restore-prev) buffer)
+		    ;; Use selected window from quit-restore-prev.
+		    (setq quit-restore-2 quit-restore-prev-2)))
 	   ;; Delete WINDOW if possible.
 	   (window--delete window nil (eq bury-or-kill 'kill)))
       ;; If the previously selected window is still alive, select it.
       (window--quit-restore-select-window quit-restore-2))
-     ((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.
+     ((or (and (listp (setq quad (nth 1 quit-restore)))
+	       (buffer-live-p (car quad))
+	       (eq (nth 3 quit-restore) buffer))
+	  (and (listp (setq quad (nth 1 quit-restore-prev)))
+	       (buffer-live-p (car quad))
+	       (eq (nth 3 quit-restore-prev) buffer)
+	       ;; Use selected window from quit-restore-prev.
+	       (setq quit-restore-2 quit-restore-prev-2)
+	       ;; We want to reset quit-restore-prev only.
+	       (setq reset-prev t)))
+      ;; Show another buffer stored in quit-restore(-prev) parameter.
       (when (and (integerp (nth 3 quad))
 		 (if (window-combined-p window)
                      (/= (nth 3 quad) (window-total-height window))
@@ -5281,15 +5304,15 @@ quit-restore-window
 	;; `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)
+      ;; Reset the quit-restore(-prev) parameter.
+      (set-window-parameter
+       window (if reset-prev 'quit-restore-prev 'quit-restore) nil)
       ;; Select old window.
       ;; If the previously selected window is still alive, select it.
       (window--quit-restore-select-window quit-restore-2))
      (t
-      ;; Show some other buffer in WINDOW and reset the quit-restore
-      ;; parameter.
-      (set-window-parameter window 'quit-restore nil)
+      ;; Show some other buffer in WINDOW and leave the
+      ;; quit-restore(-prev) parameters alone (Juri's idea).
       ;; Make sure that WINDOW is no more dedicated.
       (set-window-dedicated-p window nil)
       ;; Try to switch to a previous buffer.  Delete the window only if
@@ -5297,10 +5320,7 @@ quit-restore-window
       (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))
-      ;; If the previously selected window is still alive, select it.
-      (window--quit-restore-select-window quit-restore-2))))
-
+        (window--delete window nil (eq bury-or-kill 'kill)))))
     ;; Deal with the buffer.
     (cond
      ((not (buffer-live-p buffer)))
@@ -5342,12 +5362,20 @@ quit-windows-on
   (let ((buffer (window-normalize-buffer buffer-or-name))
 	;; Handle the "inverted" meaning of the FRAME argument wrt other
 	;; `window-list-1' based function.
-	(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)
-	  (quit-window kill window)
-	;; If a window doesn't show BUFFER, unrecord BUFFER in it.
-	(unrecord-window-buffer window buffer)))))
+	(frames (cond ((not frame) t) ((eq frame t) nil) (t frame))))
+    (dolist (window (window-list-1 nil nil frames))
+      (when (eq (window-buffer window) buffer)
+	(quit-restore-window window kill))
+      ;; Remove quit-restore(-prev) entries that mention BUFFER.
+      (let ((quit-restore (window-parameter window 'quit-restore))
+	    (quit-restore-prev (window-parameter window 'quit-restore-prev)))
+	(when (eq buffer (nth 3 quit-restore-prev))
+	  (set-window-parameter window 'quit-restore-prev nil))
+	(when (eq buffer (nth 3 quit-restore))
+	  (set-window-parameter
+	   window 'quit-restore (window-parameter window 'quit-restore-prev))))
+      ;; Unrecord BUFFER in this window.
+      (unrecord-window-buffer window buffer))))
 \f
 (defun window--combination-resizable (parent &optional horizontal)
   "Return number of pixels recoverable from height of window PARENT.
@@ -6673,7 +6701,7 @@ display-buffer-record-window
       ;; WINDOW shows another buffer.
       (with-current-buffer (window-buffer window)
 	(set-window-parameter
-	 window 'quit-restore
+	 window 'quit-restore-prev
 	 (list 'other
 	       ;; A quadruple of WINDOW's buffer, start, point and height.
 	       (list (current-buffer) (window-start window)

^ permalink raw reply related	[flat|nested] 15+ messages in thread

* bug#59862: quit-restore per window buffer
  2024-06-03  9:34   ` martin rudalics via Bug reports for GNU Emacs, the Swiss army knife of text editors
@ 2024-06-03  9:53     ` martin rudalics via Bug reports for GNU Emacs, the Swiss army knife of text editors
  2024-06-03 16:09       ` martin rudalics via Bug reports for GNU Emacs, the Swiss army knife of text editors
  0 siblings, 1 reply; 15+ messages in thread
From: martin rudalics via Bug reports for GNU Emacs, the Swiss army knife of text editors @ 2024-06-03  9:53 UTC (permalink / raw)
  To: Juri Linkov, 59862

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

The patch had an all to obvious bug when killing a buffer.  Try the one
attached here instead.  But I have to think for a better solution in
'replace-buffer-in-windows'.

martin

[-- Attachment #2: quit-restore.diff --]
[-- Type: text/x-patch, Size: 10603 bytes --]

diff --git a/lisp/window.el b/lisp/window.el
index b014be6a7cf..ae30c50cddb 100644
--- a/lisp/window.el
+++ b/lisp/window.el
@@ -4522,10 +4522,11 @@ unrecord-window-buffer
 WINDOW."
   (let* ((window (window-normalize-window window t))
 	 (buffer (or buffer (window-buffer window))))
-    (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 (buffer-live-p buffer)
+      (set-window-prev-buffers
+       window (assq-delete-all buffer (window-prev-buffers window)))
+      (set-window-next-buffers
+       window (delq buffer (window-next-buffers window))))))
 
 (defun set-window-buffer-start-and-point (window buffer &optional start point)
   "Set WINDOW's buffer to BUFFER.
@@ -5108,8 +5109,8 @@ delete-windows-on
   (let ((buffer (window-normalize-buffer buffer-or-name))
 	;; Handle the "inverted" meaning of the FRAME argument wrt other
 	;; `window-list-1' based function.
-	(all-frames (cond ((not frame) t) ((eq frame t) nil) (t frame))))
-    (dolist (window (window-list-1 nil nil all-frames))
+	(frames (cond ((not frame) t) ((eq frame t) nil) (t frame))))
+    (dolist (window (window-list-1 nil 'nomini frames))
       (if (eq (window-buffer window) buffer)
 	  (let ((deletable (window-deletable-p window))
                 (dedicated (window-dedicated-p window)))
@@ -5121,12 +5122,18 @@ delete-windows-on
 	      ;; Delete window.
 	      (delete-window window))
 	     (t
-	      ;; In window switch to previous buffer.
-	      (set-window-dedicated-p window nil)
-	      (switch-to-prev-buffer window 'bury)
-              ;; Restore the dedicated 'side' flag.
-              (when (eq dedicated 'side)
-                (set-window-dedicated-p window 'side)))))
+	      (when (eq (window-buffer window) buffer)
+		(quit-restore-window window))
+	      ;; Remove quit-restore(-prev) entries that mention BUFFER.
+	      (let ((quit-restore (window-parameter window 'quit-restore))
+		    (quit-restore-prev (window-parameter window 'quit-restore-prev)))
+		(when (eq buffer (nth 3 quit-restore-prev))
+		  (set-window-parameter window 'quit-restore-prev nil))
+		(when (eq buffer (nth 3 quit-restore))
+		  (set-window-parameter
+		   window 'quit-restore (window-parameter window 'quit-restore-prev))))
+	      ;; Unrecord BUFFER in this window.
+	      (unrecord-window-buffer window buffer))))
 	;; If a window doesn't show BUFFER, unrecord BUFFER in it.
 	(unrecord-window-buffer window buffer)))))
 
@@ -5146,17 +5153,18 @@ replace-buffer-in-windows
   (interactive "bBuffer to replace: ")
   (let ((buffer (window-normalize-buffer buffer-or-name)))
     (dolist (window (window-list-1 nil nil t))
-      (if (eq (window-buffer window) buffer)
-          ;; Delete a dedicated window unless it is a side window.
-          (let ((dedicated-side (eq (window-dedicated-p window) 'side)))
-            (when (or dedicated-side (not (window--delete window t t)))
-              ;; Switch to another buffer in that 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)))))
+      (when (eq (window-buffer window) buffer)
+	(quit-restore-window window 'kill))
+      ;; Remove quit-restore(-prev) entries that mention BUFFER.
+      (let ((quit-restore (window-parameter window 'quit-restore))
+	    (quit-restore-prev (window-parameter window 'quit-restore-prev)))
+	(when (eq buffer (nth 3 quit-restore-prev))
+	  (set-window-parameter window 'quit-restore-prev nil))
+	(when (eq buffer (nth 3 quit-restore))
+	  (set-window-parameter
+	   window 'quit-restore (window-parameter window 'quit-restore-prev))))
+      ;; Unrecord BUFFER in this window.
+      (unrecord-window-buffer window buffer))))
 
 (defcustom quit-window-hook nil
   "Hook run before performing any other actions in the `quit-window' command."
@@ -5210,13 +5218,15 @@ quit-restore-window
   (setq window (window-normalize-window window t))
   (let* ((buffer (window-buffer window))
 	 (quit-restore (window-parameter window 'quit-restore))
+	 (quit-restore-prev (window-parameter window 'quit-restore-prev))
 	 (quit-restore-2 (nth 2 quit-restore))
+	 (quit-restore-prev-2 (nth 2 quit-restore-prev))
          (prev-buffer (catch 'prev-buffer
                         (dolist (buf (window-prev-buffers window))
                           (unless (eq (car buf) buffer)
                             (throw 'prev-buffer (car buf))))))
          (dedicated (window-dedicated-p window))
-	 quad entry)
+	 quad entry reset-prev)
     (cond
      ;; First try to delete dedicated windows that are not side windows.
      ((and dedicated (not (eq dedicated 'side))
@@ -5230,21 +5240,35 @@ quit-restore-window
       ;; If the previously selected window is still alive, select it.
       (window--quit-restore-select-window quit-restore-2))
      ((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)
+	   (or (and (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))
+	       (and (or (eq (nth 1 quit-restore-prev) 'frame)
+			(and (eq (nth 1 quit-restore-prev) 'window)
+			     (not (eq window (frame-root-window window)))))
+		    (eq (nth 3 quit-restore-prev) buffer)
+		    ;; Use selected window from quit-restore-prev.
+		    (setq quit-restore-2 quit-restore-prev-2)))
 	   ;; Delete WINDOW if possible.
 	   (window--delete window nil (eq bury-or-kill 'kill)))
       ;; If the previously selected window is still alive, select it.
       (window--quit-restore-select-window quit-restore-2))
-     ((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.
+     ((or (and (listp (setq quad (nth 1 quit-restore)))
+	       (buffer-live-p (car quad))
+	       (eq (nth 3 quit-restore) buffer))
+	  (and (listp (setq quad (nth 1 quit-restore-prev)))
+	       (buffer-live-p (car quad))
+	       (eq (nth 3 quit-restore-prev) buffer)
+	       ;; Use selected window from quit-restore-prev.
+	       (setq quit-restore-2 quit-restore-prev-2)
+	       ;; We want to reset quit-restore-prev only.
+	       (setq reset-prev t)))
+      ;; Show another buffer stored in quit-restore(-prev) parameter.
       (when (and (integerp (nth 3 quad))
 		 (if (window-combined-p window)
                      (/= (nth 3 quad) (window-total-height window))
@@ -5281,15 +5305,15 @@ quit-restore-window
 	;; `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)
+      ;; Reset the quit-restore(-prev) parameter.
+      (set-window-parameter
+       window (if reset-prev 'quit-restore-prev 'quit-restore) nil)
       ;; Select old window.
       ;; If the previously selected window is still alive, select it.
       (window--quit-restore-select-window quit-restore-2))
      (t
-      ;; Show some other buffer in WINDOW and reset the quit-restore
-      ;; parameter.
-      (set-window-parameter window 'quit-restore nil)
+      ;; Show some other buffer in WINDOW and leave the
+      ;; quit-restore(-prev) parameters alone (Juri's idea).
       ;; Make sure that WINDOW is no more dedicated.
       (set-window-dedicated-p window nil)
       ;; Try to switch to a previous buffer.  Delete the window only if
@@ -5297,10 +5321,7 @@ quit-restore-window
       (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))
-      ;; If the previously selected window is still alive, select it.
-      (window--quit-restore-select-window quit-restore-2))))
-
+        (window--delete window nil (eq bury-or-kill 'kill)))))
     ;; Deal with the buffer.
     (cond
      ((not (buffer-live-p buffer)))
@@ -5342,12 +5363,20 @@ quit-windows-on
   (let ((buffer (window-normalize-buffer buffer-or-name))
 	;; Handle the "inverted" meaning of the FRAME argument wrt other
 	;; `window-list-1' based function.
-	(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)
-	  (quit-window kill window)
-	;; If a window doesn't show BUFFER, unrecord BUFFER in it.
-	(unrecord-window-buffer window buffer)))))
+	(frames (cond ((not frame) t) ((eq frame t) nil) (t frame))))
+    (dolist (window (window-list-1 nil nil frames))
+      (when (eq (window-buffer window) buffer)
+	(quit-restore-window window kill))
+      ;; Remove quit-restore(-prev) entries that mention BUFFER.
+      (let ((quit-restore (window-parameter window 'quit-restore))
+	    (quit-restore-prev (window-parameter window 'quit-restore-prev)))
+	(when (eq buffer (nth 3 quit-restore-prev))
+	  (set-window-parameter window 'quit-restore-prev nil))
+	(when (eq buffer (nth 3 quit-restore))
+	  (set-window-parameter
+	   window 'quit-restore (window-parameter window 'quit-restore-prev))))
+      ;; Unrecord BUFFER in this window.
+      (unrecord-window-buffer window buffer))))
 \f
 (defun window--combination-resizable (parent &optional horizontal)
   "Return number of pixels recoverable from height of window PARENT.
@@ -6673,7 +6702,7 @@ display-buffer-record-window
       ;; WINDOW shows another buffer.
       (with-current-buffer (window-buffer window)
 	(set-window-parameter
-	 window 'quit-restore
+	 window 'quit-restore-prev
 	 (list 'other
 	       ;; A quadruple of WINDOW's buffer, start, point and height.
 	       (list (current-buffer) (window-start window)

^ permalink raw reply related	[flat|nested] 15+ messages in thread

* bug#59862: quit-restore per window buffer
  2024-06-03  9:53     ` martin rudalics via Bug reports for GNU Emacs, the Swiss army knife of text editors
@ 2024-06-03 16:09       ` martin rudalics via Bug reports for GNU Emacs, the Swiss army knife of text editors
  2024-06-04  6:53         ` Juri Linkov
  0 siblings, 1 reply; 15+ messages in thread
From: martin rudalics via Bug reports for GNU Emacs, the Swiss army knife of text editors @ 2024-06-03 16:09 UTC (permalink / raw)
  To: Juri Linkov, 59862

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

I revisited it once more and attach my last patch.  It also seems to
DTRT with C-x 4 d RET C-h C-t C-h C-n q q q but would still require a
longer period of testing.

martin

[-- Attachment #2: quit-restore.diff --]
[-- Type: text/x-patch, Size: 16371 bytes --]

diff --git a/lisp/window.el b/lisp/window.el
index b014be6a7cf..dcfc578ffd8 100644
--- a/lisp/window.el
+++ b/lisp/window.el
@@ -4519,13 +4519,33 @@ unrecord-window-buffer
   "Unrecord BUFFER in WINDOW.
 WINDOW must be a live window and defaults to the selected one.
 BUFFER must be a live buffer and defaults to the buffer of
-WINDOW."
+WINDOW.
+
+Make BUFFER disappear from all variables mentioned by the object of
+WINDOW.  This includes the buffers previously shwon in WINDOW as well as
+any buffers mentioned by WINDOW's `quit-restore' and `quit-restore-prev'
+parameters."
   (let* ((window (window-normalize-window window t))
-	 (buffer (or buffer (window-buffer window))))
-    (set-window-prev-buffers
-     window (assq-delete-all buffer (window-prev-buffers window)))
-    (set-window-next-buffers
-     window (delq buffer (window-next-buffers window)))))
+	 (buffer (or buffer (window-buffer window)))
+	 (quit-restore (window-parameter window 'quit-restore))
+	 (quit-restore-prev (window-parameter window 'quit-restore-prev))
+	 quad)
+    (when (buffer-live-p buffer)
+      (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 (or (eq buffer (nth 3 quit-restore-prev))
+		(and (listp (setq quad (nth 1 quit-restore-prev)))
+		     (eq (car quad) buffer)))
+	(set-window-parameter window 'quit-restore-prev nil))
+
+      (when (or (eq buffer (nth 3 quit-restore))
+		(and (listp (setq quad (nth 1 quit-restore)))
+		     (eq (car quad) buffer)))
+	(set-window-parameter
+	 window 'quit-restore (window-parameter window 'quit-restore-prev))))))
 
 (defun set-window-buffer-start-and-point (window buffer &optional start point)
   "Set WINDOW's buffer to BUFFER.
@@ -5075,21 +5095,20 @@ delete-windows-on
 use \\[universal-argument] 0 to specify all windows only on
 the current terminal's frames.
 
-If a frame's root window shows the buffer specified by
-BUFFER-OR-NAME and is dedicated to that buffer and that frame
-does not host the active minibuffer window and there is at least
-one other frame on that frame's terminal, delete that frame.
-Otherwise, do not delete a frame's root window if it shows the
-buffer specified by BUFFER-OR-NAME and do not delete any frame's
-main window showing that buffer either.  Rather, in any such
-case, call `switch-to-prev-buffer' to show another buffer in that
-window and make sure the window is no more dedicated to its
-buffer.
-
-If the buffer specified by BUFFER-OR-NAME is shown in a
-minibuffer window, do nothing for that window.  For any window
-that does not show that buffer, remove the buffer from that
-window's lists of previous and next buffers."
+If a frame's root window shows the buffer specified by BUFFER-OR-NAME
+and is dedicated to that buffer and that frame does not host the active
+minibuffer window and there is at least one other frame on that frame's
+terminal, delete that frame.  Otherwise, do not delete a frame's root
+window if it shows the buffer specified by BUFFER-OR-NAME and do not
+delete any frame's main window showing that buffer either.  Rather, in
+any such case, call `quit-restore-window' to show another buffer in that
+window and make sure the window is no more dedicated to its buffer.
+
+If the buffer specified by BUFFER-OR-NAME is shown in a minibuffer
+window, do nothing for that window.  For any window that does not show
+that buffer, remove the buffer from that window's lists of previous and
+next buffers and remove any `quit-restore' and `quit-restore-prev'
+parameters naming it."
   (interactive
    (let ((frame (cond
                  ((and (numberp current-prefix-arg)
@@ -5107,9 +5126,9 @@ delete-windows-on
            frame)))
   (let ((buffer (window-normalize-buffer buffer-or-name))
 	;; Handle the "inverted" meaning of the FRAME argument wrt other
-	;; `window-list-1' based function.
-	(all-frames (cond ((not frame) t) ((eq frame t) nil) (t frame))))
-    (dolist (window (window-list-1 nil nil all-frames))
+	;; `window-list-1' based functions.
+	(frames (cond ((not frame) t) ((eq frame t) nil) (t frame))))
+    (dolist (window (window-list-1 nil 'nomini frames))
       (if (eq (window-buffer window) buffer)
 	  (let ((deletable (window-deletable-p window))
                 (dedicated (window-dedicated-p window)))
@@ -5121,19 +5140,19 @@ delete-windows-on
 	      ;; Delete window.
 	      (delete-window window))
 	     (t
-	      ;; In window switch to previous buffer.
-	      (set-window-dedicated-p window nil)
-	      (switch-to-prev-buffer window 'bury)
-              ;; Restore the dedicated 'side' flag.
-              (when (eq dedicated 'side)
-                (set-window-dedicated-p window 'side)))))
+	      (when (eq (window-buffer window) buffer)
+		(quit-restore-window window 'bury))
+	      (when (window-live-p window)
+		;; Unrecord BUFFER in this window.
+		(unrecord-window-buffer window buffer)))))
 	;; If a window doesn't show BUFFER, unrecord BUFFER in it.
 	(unrecord-window-buffer window buffer)))))
 
 (defun replace-buffer-in-windows (&optional buffer-or-name)
   "Replace BUFFER-OR-NAME with some other buffer in all windows showing it.
 BUFFER-OR-NAME may be a buffer or the name of an existing buffer
-and defaults to the current buffer.
+and defaults to the current buffer.  Minibuffer windows are not
+considered.
 
 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
@@ -5141,21 +5160,18 @@ replace-buffer-in-windows
 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
-all window-local buffer lists."
+This function removes the buffer denoted by BUFFER-OR-NAME from all
+window-local buffer lists and removes any `quit-restore' or
+`quit-restore-prev' parameters mentioning it."
   (interactive "bBuffer to replace: ")
   (let ((buffer (window-normalize-buffer buffer-or-name)))
+    ;; Scan all windows.  We have to unrecord BUFFER in those not
+    ;; showing it.
     (dolist (window (window-list-1 nil nil t))
-      (if (eq (window-buffer window) buffer)
-          ;; Delete a dedicated window unless it is a side window.
-          (let ((dedicated-side (eq (window-dedicated-p window) 'side)))
-            (when (or dedicated-side (not (window--delete window t t)))
-              ;; Switch to another buffer in that 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.
+      (when (eq (window-buffer window) buffer)
+	(quit-restore-window window))
+      (when (window-live-p window)
+	;; Unrecord BUFFER in this window.
 	(unrecord-window-buffer window buffer)))))
 
 (defcustom quit-window-hook nil
@@ -5176,13 +5192,13 @@ quit-restore-window
   "Quit WINDOW and deal with its buffer.
 WINDOW must be a live window and defaults to the selected one.
 
-According to information stored in WINDOW's `quit-restore' window
-parameter either (1) delete WINDOW and its frame, (2) delete
-WINDOW but leave its frame alone, (3) restore the buffer
-previously shown in WINDOW, or (4) make WINDOW display some other
-buffer.  If WINDOW is not deleted, reset its `quit-restore'
-parameter to nil.  See Info node `(elisp) Quitting Windows' for
-more details.
+According to information stored in WINDOW's `quit-restore' and
+`quit-restore-prev' window parameters either (1) delete WINDOW and its
+frame, (2) delete WINDOW but leave its frame alone, (3) restore the
+buffer previously shown in WINDOW, or (4) make WINDOW display some other
+buffer.  In case (3) set any of these parameters to nil if it has been
+used to restore the previously shown buffer.  See Info node `(elisp)
+Quitting Windows' for more details.
 
 If WINDOW's dedicated flag is t, try to delete WINDOW.  If it
 equals the value `side', restore that value when WINDOW is not
@@ -5210,13 +5226,15 @@ quit-restore-window
   (setq window (window-normalize-window window t))
   (let* ((buffer (window-buffer window))
 	 (quit-restore (window-parameter window 'quit-restore))
+	 (quit-restore-prev (window-parameter window 'quit-restore-prev))
 	 (quit-restore-2 (nth 2 quit-restore))
+	 (quit-restore-prev-2 (nth 2 quit-restore-prev))
          (prev-buffer (catch 'prev-buffer
                         (dolist (buf (window-prev-buffers window))
                           (unless (eq (car buf) buffer)
                             (throw 'prev-buffer (car buf))))))
          (dedicated (window-dedicated-p window))
-	 quad entry)
+	 quad entry reset-prev)
     (cond
      ;; First try to delete dedicated windows that are not side windows.
      ((and dedicated (not (eq dedicated 'side))
@@ -5230,21 +5248,35 @@ quit-restore-window
       ;; If the previously selected window is still alive, select it.
       (window--quit-restore-select-window quit-restore-2))
      ((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)
+	   (or (and (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))
+	       (and (or (eq (nth 1 quit-restore-prev) 'frame)
+			(and (eq (nth 1 quit-restore-prev) 'window)
+			     (not (eq window (frame-root-window window)))))
+		    (eq (nth 3 quit-restore-prev) buffer)
+		    ;; Use selected window from quit-restore-prev.
+		    (setq quit-restore-2 quit-restore-prev-2)))
 	   ;; Delete WINDOW if possible.
 	   (window--delete window nil (eq bury-or-kill 'kill)))
       ;; If the previously selected window is still alive, select it.
       (window--quit-restore-select-window quit-restore-2))
-     ((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.
+     ((or (and (listp (setq quad (nth 1 quit-restore-prev)))
+	       (buffer-live-p (car quad))
+	       (eq (nth 3 quit-restore-prev) buffer)
+	       ;; Use selected window from quit-restore-prev.
+	       (setq quit-restore-2 quit-restore-prev-2)
+	       ;; We want to reset quit-restore-prev only.
+	       (setq reset-prev t))
+	  (and (listp (setq quad (nth 1 quit-restore)))
+	       (buffer-live-p (car quad))
+	       (eq (nth 3 quit-restore) buffer)))
+      ;; Show another buffer stored in quit-restore(-prev) parameter.
       (when (and (integerp (nth 3 quad))
 		 (if (window-combined-p window)
                      (/= (nth 3 quad) (window-total-height window))
@@ -5281,15 +5313,18 @@ quit-restore-window
 	;; `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)
+      ;; Reset the quit-restore(-prev) parameter.
+      (set-window-parameter window 'quit-restore-prev nil)
+      (unless reset-prev
+	;; If quit-restore-prev was not used, reset the quit-restore
+	;; parameter
+	(set-window-parameter window 'quit-restore nil))
       ;; Select old window.
       ;; If the previously selected window is still alive, select it.
       (window--quit-restore-select-window quit-restore-2))
      (t
-      ;; Show some other buffer in WINDOW and reset the quit-restore
-      ;; parameter.
-      (set-window-parameter window 'quit-restore nil)
+      ;; Show some other buffer in WINDOW and leave the
+      ;; quit-restore(-prev) parameters alone (Juri's idea).
       ;; Make sure that WINDOW is no more dedicated.
       (set-window-dedicated-p window nil)
       ;; Try to switch to a previous buffer.  Delete the window only if
@@ -5297,10 +5332,7 @@ quit-restore-window
       (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))
-      ;; If the previously selected window is still alive, select it.
-      (window--quit-restore-select-window quit-restore-2))))
-
+        (window--delete window nil (eq bury-or-kill 'kill)))))
     ;; Deal with the buffer.
     (cond
      ((not (buffer-live-p buffer)))
@@ -5336,17 +5368,21 @@ quit-windows-on
 BUFFER-OR-NAME.  Optional argument FRAME is handled as by
 `delete-windows-on'.
 
-This function calls `quit-window' on all candidate windows
-showing BUFFER-OR-NAME."
+This function calls `quit-restore-window' on all candidate windows
+showing BUFFER-OR-NAME.  In addition, it removes the buffer denoted by
+BUFFER-OR-NAME from all window-local buffer lists and removes any
+`quit-restore' or `quit-restore-prev' parameters mentioning it."
   (interactive "bQuit windows on (buffer):\nP")
   (let ((buffer (window-normalize-buffer buffer-or-name))
 	;; Handle the "inverted" meaning of the FRAME argument wrt other
-	;; `window-list-1' based function.
-	(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)
-	  (quit-window kill window)
-	;; If a window doesn't show BUFFER, unrecord BUFFER in it.
+	;; `window-list' based function.
+	(frames (cond ((not frame) t) ((eq frame t) nil) (t frame))))
+    (dolist (window (window-list-1 nil nil frames))
+      (when (eq (window-buffer window) buffer)
+	(quit-restore-window window kill))
+
+      (when (window-live-p window)
+	;; Unrecord BUFFER in this window.
 	(unrecord-window-buffer window buffer)))))
 \f
 (defun window--combination-resizable (parent &optional horizontal)
@@ -6656,7 +6692,13 @@ display-buffer-record-window
 previously shown in the window, that buffer's window start and
 window point, and the window's height.  The third element is the
 window selected at the time the parameter was created.  The
-fourth element is BUFFER."
+fourth element is BUFFER.
+
+If TYPE is `reuse', BUFFER is different from the one currently displayed
+in WINDOW, and WINDOW already has a `quit-restore' parameter, install or
+update a `quit-restore-prev' parameter for this window that allows to
+quit WINDOW in a similar fashion but remembers the very first in a
+series of buffer display operations for this window."
   (cond
    ((eq type 'reuse)
     (if (eq (window-buffer window) buffer)
@@ -6673,7 +6715,7 @@ display-buffer-record-window
       ;; WINDOW shows another buffer.
       (with-current-buffer (window-buffer window)
 	(set-window-parameter
-	 window 'quit-restore
+	 window 'quit-restore-prev
 	 (list 'other
 	       ;; A quadruple of WINDOW's buffer, start, point and height.
 	       (list (current-buffer) (window-start window)
@@ -9103,6 +9145,9 @@ switch-to-buffer
 as prescribed by the option `switch-to-buffer-preserve-window-point'.
 Otherwise, these are left alone.
 
+In either case, call `display-buffer-record-window' to avoid disrupting
+a sequence of `display-buffer' operations using this window.
+
 Return the buffer switched to."
   (interactive
    (let ((force-same-window
@@ -9158,6 +9203,11 @@ switch-to-buffer
                                     buffer))
 	       (displayed (and (eq preserve-win-point 'already-displayed)
 			       (get-buffer-window buffer 0))))
+
+	  ;; Make sure quitting the window works.
+	  (unless switch-to-buffer-obey-display-actions
+	    (display-buffer-record-window 'reuse (selected-window) buffer))
+
 	  (set-window-buffer nil buffer)
 	  (when (and entry (or (eq preserve-win-point t) displayed))
 	    ;; Try to restore start and point of buffer in the selected

^ permalink raw reply related	[flat|nested] 15+ messages in thread

* bug#59862: quit-restore per window buffer
  2024-06-03 16:09       ` martin rudalics via Bug reports for GNU Emacs, the Swiss army knife of text editors
@ 2024-06-04  6:53         ` Juri Linkov
  2024-06-05 16:56           ` Juri Linkov
  2024-06-11  6:52           ` Juri Linkov
  0 siblings, 2 replies; 15+ messages in thread
From: Juri Linkov @ 2024-06-04  6:53 UTC (permalink / raw)
  To: martin rudalics; +Cc: 59862

> I revisited it once more and attach my last patch.  It also seems to
> DTRT with C-x 4 d RET C-h C-t C-h C-n q q q but would still require a
> longer period of testing.

Thanks, I started testing.





^ permalink raw reply	[flat|nested] 15+ messages in thread

* bug#59862: quit-restore per window buffer
  2024-06-04  6:53         ` Juri Linkov
@ 2024-06-05 16:56           ` Juri Linkov
  2024-06-11  6:52           ` Juri Linkov
  1 sibling, 0 replies; 15+ messages in thread
From: Juri Linkov @ 2024-06-05 16:56 UTC (permalink / raw)
  To: martin rudalics; +Cc: 59862

>> I revisited it once more and attach my last patch.  It also seems to
>> DTRT with C-x 4 d RET C-h C-t C-h C-n q q q but would still require a
>> longer period of testing.
>
> Thanks, I started testing.

The testing showed that now quit-restore works much better,
and even supports tabs.  Thank you very much.





^ permalink raw reply	[flat|nested] 15+ messages in thread

* bug#59862: quit-restore per window buffer
  2024-06-04  6:53         ` Juri Linkov
  2024-06-05 16:56           ` Juri Linkov
@ 2024-06-11  6:52           ` Juri Linkov
  2024-06-12  8:57             ` martin rudalics via Bug reports for GNU Emacs, the Swiss army knife of text editors
  1 sibling, 1 reply; 15+ messages in thread
From: Juri Linkov @ 2024-06-11  6:52 UTC (permalink / raw)
  To: martin rudalics; +Cc: 59862

>> I revisited it once more and attach my last patch.  It also seems to
>> DTRT with C-x 4 d RET C-h C-t C-h C-n q q q but would still require a
>> longer period of testing.
>
> Thanks, I started testing.

I found one difference between frames and tabs:

C-x 5 5   ;; other-frame-prefix
C-h i     ;; info
q         ;; quit-window
This deletes the frame.

C-x 5 5   ;; other-frame-prefix
C-h i     ;; info
C-h e     ;; view-echo-area-messages
q         ;; quit-window
This doesn't delete the frame because another buffer was used.

So frames are handled correctly.  But not tabs:

C-x t t   ;; other-tab-prefix
C-h i     ;; info
q         ;; quit-window
This correctly closes the tab.

C-x 5 5   ;; other-tab-prefix
C-h i     ;; info
C-h e     ;; view-echo-area-messages
q         ;; quit-window
But this also closes the tab, this is a destructive operation,
because the user has another buffer shown in the tab.





^ permalink raw reply	[flat|nested] 15+ messages in thread

* bug#59862: quit-restore per window buffer
  2024-06-11  6:52           ` Juri Linkov
@ 2024-06-12  8:57             ` martin rudalics via Bug reports for GNU Emacs, the Swiss army knife of text editors
  2024-06-13  6:47               ` Juri Linkov
  0 siblings, 1 reply; 15+ messages in thread
From: martin rudalics via Bug reports for GNU Emacs, the Swiss army knife of text editors @ 2024-06-12  8:57 UTC (permalink / raw)
  To: Juri Linkov; +Cc: 59862

 > C-x 5 5   ;; other-frame-prefix
 > C-h i     ;; info
 > q         ;; quit-window
 > This deletes the frame.
 >
 > C-x 5 5   ;; other-frame-prefix
 > C-h i     ;; info
 > C-h e     ;; view-echo-area-messages
 > q         ;; quit-window
 > This doesn't delete the frame because another buffer was used.

Here it doesn't delete the frame because it contains two windows.
'quit-window' deletes the *info* window and the *Messages* window
remains.  When I do C-x o q q instead it first shows the *info* window
only and then deletes the frame just as I would expect.

BTW 'view-echo-area-messages' has a bug: The 'goto-char' moves point in
the *Messages* buffer in an undocumented manner.  It should be written
as:

(defun view-echo-area-messages ()
   "View the log of recent echo-area messages: the `*Messages*' buffer.
The number of messages retained in that buffer is specified by
the variable `message-log-max'."
   (interactive)
   (when-let ((win (display-buffer (messages-buffer))))
     (set-window-point win (point-max))
     win))

 > So frames are handled correctly.  But not tabs:
 >
 > C-x t t   ;; other-tab-prefix
 > C-h i     ;; info
 > q         ;; quit-window
 > This correctly closes the tab.
 >
 > C-x 5 5   ;; other-tab-prefix

I suppose you mean C-x t t here.

 > C-h i     ;; info
 > C-h e     ;; view-echo-area-messages
 > q         ;; quit-window
 > But this also closes the tab, this is a destructive operation,
 > because the user has another buffer shown in the tab.

Here I get two windows.  The 'quit-restore' parameter of the *info*
window is

(tab tab #<window 7 on *info*> #<buffer *info*>)

that of the *Messages* window is

(window window #<window 7 on *info*> #<buffer *Messages*>).

So if I now do q in the *info* window, it will run 'tab-bar-close-tab'.
It's up to that function to DTRT here.

martin





^ permalink raw reply	[flat|nested] 15+ messages in thread

* bug#59862: quit-restore per window buffer
  2024-06-12  8:57             ` martin rudalics via Bug reports for GNU Emacs, the Swiss army knife of text editors
@ 2024-06-13  6:47               ` Juri Linkov
  2024-06-13  8:21                 ` martin rudalics via Bug reports for GNU Emacs, the Swiss army knife of text editors
  0 siblings, 1 reply; 15+ messages in thread
From: Juri Linkov @ 2024-06-13  6:47 UTC (permalink / raw)
  To: martin rudalics; +Cc: 59862

>> C-x 5 5   ;; other-frame-prefix
>> C-h i     ;; info
>> q         ;; quit-window
>> This deletes the frame.
>>
>> C-x 5 5   ;; other-frame-prefix
>> C-h i     ;; info
>> C-h e     ;; view-echo-area-messages
>> q         ;; quit-window
>> This doesn't delete the frame because another buffer was used.
>
> Here it doesn't delete the frame because it contains two windows.
> 'quit-window' deletes the *info* window and the *Messages* window
> remains.  When I do C-x o q q instead it first shows the *info* window
> only and then deletes the frame just as I would expect.

The current behavior for frames looks correct.

> BTW 'view-echo-area-messages' has a bug: The 'goto-char' moves point in
> the *Messages* buffer in an undocumented manner.  It should be written
> as:
>
> (defun view-echo-area-messages ()
>   "View the log of recent echo-area messages: the `*Messages*' buffer.
> The number of messages retained in that buffer is specified by
> the variable `message-log-max'."
>   (interactive)
>   (when-let ((win (display-buffer (messages-buffer))))
>     (set-window-point win (point-max))
>     win))

Agreed.

>> So frames are handled correctly.  But not tabs:
>>
>> C-x t t   ;; other-tab-prefix
>> C-h i     ;; info
>> q         ;; quit-window
>> This correctly closes the tab.
>>
>> C-x t t   ;; other-tab-prefix
>> C-h i     ;; info
>> C-h e     ;; view-echo-area-messages
>> q         ;; quit-window
>> But this also closes the tab, this is a destructive operation,
>> because the user has another buffer shown in the tab.
>
> Here I get two windows.  The 'quit-restore' parameter of the *info*
> window is
>
> (tab tab #<window 7 on *info*> #<buffer *info*>)
>
> that of the *Messages* window is
>
> (window window #<window 7 on *info*> #<buffer *Messages*>).
>
> So if I now do q in the *info* window, it will run 'tab-bar-close-tab'.
> It's up to that function to DTRT here.

Could you show how the frame function corresponding to 'tab-bar-close-tab'
(I suppose it's 'delete-frame'?) detects that where are more than 1 window
on the frame, then it refuses to delete the frame.  Then same code could be
used in 'tab-bar-close-tab'.





^ permalink raw reply	[flat|nested] 15+ messages in thread

* bug#59862: quit-restore per window buffer
  2024-06-13  6:47               ` Juri Linkov
@ 2024-06-13  8:21                 ` martin rudalics via Bug reports for GNU Emacs, the Swiss army knife of text editors
  2024-06-14 17:35                   ` Juri Linkov
  0 siblings, 1 reply; 15+ messages in thread
From: martin rudalics via Bug reports for GNU Emacs, the Swiss army knife of text editors @ 2024-06-13  8:21 UTC (permalink / raw)
  To: Juri Linkov; +Cc: 59862

 > Could you show how the frame function corresponding to 'tab-bar-close-tab'
 > (I suppose it's 'delete-frame'?)

Hardly.  'delete-frame' unconditionally deletes a frame regardless of
how many windows it displays unless it is the last frame, a surrogate
minibuffer frame or some other special condition holds.

 > detects that where are more than 1 window
 > on the frame, then it refuses to delete the frame.  Then same code could be
 > used in 'tab-bar-close-tab'.

The check is in 'window-deletable-p' which may return 'frame' iff WINDOW
is its frame's root window.  Maybe you also want to check whether the
second element of 'quit-restore' is 'frame' but that I cannot tell.

Note in this context that 'window-deletable-p' does not cover all
special cases handled by 'delete-frame' - things like the
'delete-before' parameter or the 'delete-frame-functions' hook.  So far,
this apparently has not caused any problems though.

martin





^ permalink raw reply	[flat|nested] 15+ messages in thread

* bug#59862: quit-restore per window buffer
  2024-06-13  8:21                 ` martin rudalics via Bug reports for GNU Emacs, the Swiss army knife of text editors
@ 2024-06-14 17:35                   ` Juri Linkov
  2024-06-15  8:41                     ` martin rudalics via Bug reports for GNU Emacs, the Swiss army knife of text editors
  0 siblings, 1 reply; 15+ messages in thread
From: Juri Linkov @ 2024-06-14 17:35 UTC (permalink / raw)
  To: martin rudalics; +Cc: 59862

> The check is in 'window-deletable-p' which may return 'frame' iff WINDOW
> is its frame's root window.  Maybe you also want to check whether the
> second element of 'quit-restore' is 'frame' but that I cannot tell.

Ok, so here is the corresponding fix for the case:

  C-x t t   ;; other-tab-prefix
  C-h i     ;; info
  C-h e     ;; view-echo-area-messages
  q         ;; quit-window

diff --git a/lisp/window.el b/lisp/window.el
index b7bd59bc813..f206153e017 100644
--- a/lisp/window.el
+++ b/lisp/window.el
@@ -5243,7 +5258,10 @@ quit-restore-window
       (window--quit-restore-select-window quit-restore-2))
      ((and (not prev-buffer)
 	   (eq (nth 1 quit-restore) 'tab)
-	   (eq (nth 3 quit-restore) buffer))
+	   (eq (nth 3 quit-restore) buffer)
+	   (< (seq-count (lambda (w) (window-parameter w 'quit-restore))
+		         (window-list-1 nil 'nomini))
+              2))
       (tab-bar-close-tab)
       ;; If the previously selected window is still alive, select it.
       (window--quit-restore-select-window quit-restore-2))





^ permalink raw reply related	[flat|nested] 15+ messages in thread

* bug#59862: quit-restore per window buffer
  2024-06-14 17:35                   ` Juri Linkov
@ 2024-06-15  8:41                     ` martin rudalics via Bug reports for GNU Emacs, the Swiss army knife of text editors
  2024-06-16 16:50                       ` Juri Linkov
  0 siblings, 1 reply; 15+ messages in thread
From: martin rudalics via Bug reports for GNU Emacs, the Swiss army knife of text editors @ 2024-06-15  8:41 UTC (permalink / raw)
  To: Juri Linkov; +Cc: 59862


  +	   (< (seq-count (lambda (w) (window-parameter w 'quit-restore))

The entire clause in 'quit-restore-window' governed by

	   (eq (nth 1 quit-restore) 'tab)

needs a comment on what it is supposed to restore.  Basically, you check
here whether WINDOW is the only window with a 'quit-restore' parameter.
Is it that what you really want?  If a window has a 'quit-restore'
parameter whose second element is 'tab' does that mean that all other
windows on the same frame must also have such a parameter?  What happens
if there are other windows without a 'quit-restore' parameter?

As designed, the 'quit-restore' parameter can cause the deletion of its
window only if there are other windows on the same frame.  It can cause
the deletion of its frame only if there is no other window on its frame.
Are these principles preserved by your patch?

 > +		         (window-list-1 nil 'nomini))

I suppose that nil is not correct here when WINDOW is not on the
selected frame.  I'd rather use that window instead here.

 > +              2))
 >         (tab-bar-close-tab)
 >         ;; If the previously selected window is still alive, select it.
 >         (window--quit-restore-select-window quit-restore-2))

martin





^ permalink raw reply	[flat|nested] 15+ messages in thread

* bug#59862: quit-restore per window buffer
  2024-06-15  8:41                     ` martin rudalics via Bug reports for GNU Emacs, the Swiss army knife of text editors
@ 2024-06-16 16:50                       ` Juri Linkov
  2024-06-17 14:48                         ` martin rudalics via Bug reports for GNU Emacs, the Swiss army knife of text editors
  0 siblings, 1 reply; 15+ messages in thread
From: Juri Linkov @ 2024-06-16 16:50 UTC (permalink / raw)
  To: martin rudalics; +Cc: 59862

>  +	   (< (seq-count (lambda (w) (window-parameter w 'quit-restore))
>
> The entire clause in 'quit-restore-window' governed by
>
> 	   (eq (nth 1 quit-restore) 'tab)
>
> needs a comment on what it is supposed to restore.

So here is a comment:

diff --git a/lisp/window.el b/lisp/window.el
index b7bd59bc813..5b782c93098 100644
--- a/lisp/window.el
+++ b/lisp/window.el
@@ -5241,9 +5246,13 @@ quit-restore-window
            (window--delete window 'dedicated (eq bury-or-kill 'kill)))
       ;; If the previously selected window is still alive, select it.
       (window--quit-restore-select-window quit-restore-2))
+     ;; Close the tab if it doesn't contain more explicitly created windows.
      ((and (not prev-buffer)
 	   (eq (nth 1 quit-restore) 'tab)
-	   (eq (nth 3 quit-restore) buffer))
+	   (eq (nth 3 quit-restore) buffer)
+	   (< (seq-count (lambda (w) (window-parameter w 'quit-restore))
+			 (window-list-1 nil 'nomini))
+	      2))
       (tab-bar-close-tab)
       ;; If the previously selected window is still alive, select it.
       (window--quit-restore-select-window quit-restore-2))

> Basically, you check
> here whether WINDOW is the only window with a 'quit-restore' parameter.
> Is it that what you really want?  If a window has a 'quit-restore'
> parameter whose second element is 'tab' does that mean that all other
> windows on the same frame must also have such a parameter?  What happens
> if there are other windows without a 'quit-restore' parameter?

If there are other windows without a 'quit-restore' parameter,
this means that they were created implicitly without a direct
user action.  So the tab can be closed in this case.  There are
not many commands that create windows without a 'quit-restore'
parameter.  For example, Gnus creates several windows, and
the tab should be closed when the user closes the first window.

> As designed, the 'quit-restore' parameter can cause the deletion of its
> window only if there are other windows on the same frame.  It can cause
> the deletion of its frame only if there is no other window on its frame.
> Are these principles preserved by your patch?

Yes, the patch closes the tab only if there is no other window on the tab
explicitly created by user.

>> +		         (window-list-1 nil 'nomini))
>
> I suppose that nil is not correct here when WINDOW is not on the
> selected frame.  I'd rather use that window instead here.

The current code doesn't support non-selected frames
as 'tab-bar-close-tab' below shows without a frame argument:

>> +              2))
>>         (tab-bar-close-tab)
>>         ;; If the previously selected window is still alive, select it.
>>         (window--quit-restore-select-window quit-restore-2))





^ permalink raw reply related	[flat|nested] 15+ messages in thread

* bug#59862: quit-restore per window buffer
  2024-06-16 16:50                       ` Juri Linkov
@ 2024-06-17 14:48                         ` martin rudalics via Bug reports for GNU Emacs, the Swiss army knife of text editors
  0 siblings, 0 replies; 15+ messages in thread
From: martin rudalics via Bug reports for GNU Emacs, the Swiss army knife of text editors @ 2024-06-17 14:48 UTC (permalink / raw)
  To: Juri Linkov; +Cc: 59862

 > +	   (< (seq-count (lambda (w) (window-parameter w 'quit-restore))
 > +			 (window-list-1 nil 'nomini))

As I said before nil as argument for 'window-list-1' is not correct.
'window--delete' can be called by 'quit-windows-on' which has to work
for quitting any window on any frame.

 >>> +		         (window-list-1 nil 'nomini))
 >>
 >> I suppose that nil is not correct here when WINDOW is not on the
 >> selected frame.  I'd rather use that window instead here.
 >
 > The current code doesn't support non-selected frames
 > as 'tab-bar-close-tab' below shows without a frame argument:

Then we can't use it.  Please have a look at the patch I sent earlier.
The tab-bar code would have to add a function to
'window-deletable-functions' provided some tab-bar option says to do
that.

Thanks, martin





^ permalink raw reply	[flat|nested] 15+ messages in thread

end of thread, other threads:[~2024-06-17 14:48 UTC | newest]

Thread overview: 15+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2022-12-06 17:32 bug#59862: quit-restore per window buffer Juri Linkov
2024-06-02  6:45 ` Juri Linkov
2024-06-03  9:34   ` martin rudalics via Bug reports for GNU Emacs, the Swiss army knife of text editors
2024-06-03  9:53     ` martin rudalics via Bug reports for GNU Emacs, the Swiss army knife of text editors
2024-06-03 16:09       ` martin rudalics via Bug reports for GNU Emacs, the Swiss army knife of text editors
2024-06-04  6:53         ` Juri Linkov
2024-06-05 16:56           ` Juri Linkov
2024-06-11  6:52           ` Juri Linkov
2024-06-12  8:57             ` martin rudalics via Bug reports for GNU Emacs, the Swiss army knife of text editors
2024-06-13  6:47               ` Juri Linkov
2024-06-13  8:21                 ` martin rudalics via Bug reports for GNU Emacs, the Swiss army knife of text editors
2024-06-14 17:35                   ` Juri Linkov
2024-06-15  8:41                     ` martin rudalics via Bug reports for GNU Emacs, the Swiss army knife of text editors
2024-06-16 16:50                       ` Juri Linkov
2024-06-17 14:48                         ` martin rudalics via Bug reports for GNU Emacs, the Swiss army knife of text editors

Code repositories for project(s) associated with this external index

	https://git.savannah.gnu.org/cgit/emacs.git
	https://git.savannah.gnu.org/cgit/emacs/org-mode.git

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.