bug#59862: quit-restore per window buffer

[martin rudalics via "Bug reports for GNU Emacs, the Swiss army knife of text editors" <bug-gnu-emacs@gnu.org>]

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