* 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; 45+ 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] 45+ 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; 45+ 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] 45+ 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; 45+ 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] 45+ 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; 45+ 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] 45+ 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; 45+ 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] 45+ 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; 45+ 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] 45+ 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; 45+ 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] 45+ 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; 45+ 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] 45+ 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; 45+ 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] 45+ 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; 45+ 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] 45+ 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; 45+ 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] 45+ 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; 45+ 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] 45+ 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; 45+ 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] 45+ 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 2024-07-08 16:49 ` martin rudalics via Bug reports for GNU Emacs, the Swiss army knife of text editors 0 siblings, 2 replies; 45+ 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] 45+ 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 2024-07-08 16:49 ` martin rudalics via Bug reports for GNU Emacs, the Swiss army knife of text editors 1 sibling, 0 replies; 45+ 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] 45+ 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 @ 2024-07-08 16:49 ` martin rudalics via Bug reports for GNU Emacs, the Swiss army knife of text editors 2024-07-09 6:58 ` Juri Linkov 1 sibling, 1 reply; 45+ messages in thread From: martin rudalics via Bug reports for GNU Emacs, the Swiss army knife of text editors @ 2024-07-08 16:49 UTC (permalink / raw) To: Juri Linkov; +Cc: 59862 [-- Attachment #1: Type: text/plain, Size: 3267 bytes --] It took me some time to rewrite the code for this since I found a couple of leaks in the window handling code which I tried to plug now. In a nutshell, the new patch is supposed to fix the following issues. - I introduced a new variable called 'kill-buffer-quit-windows'. If non-nil, it makes killing a buffer call 'quit-restore-window' for each window showing the buffer thus implementing the behavior you want. If nil, killing a buffer works as before apart from the fixes I added below. - The new window parameter type 'quit-restore-prev' that was already in the previous patch handles most scenarios of traversing an entire chain of buffer display operations back to the one that initially created a window or a frame. The only thing it does not handle is to restore the previously selected window for all but the first and last such operation. - As in the previous patch I remove a buffer from all live windows' previous/next buffers and quit-restore(-prev) parameters when killing it. In the present patch I remove it also from dead windows (windows presumably stored in window configurations if they survived at least one collection cycle after their deletion). This should eliminate the necessity to scan these lists separately in the mark phase of the collector. - The old code erroneously has an internal window share the lists of previous/next buffers of a live window whenever that internal window is created by splitting the live window. If the live window is subsequently deleted but the internal window persists, these lists will continue to exist until the internal window is deleted. Since you cannot switch buffers in internal windows, these lists are completely useless while increasing memory overhead. The patch fixes this leak. Dead windows are kept in a weak hash table, adding a window when it is deleted and removing it when restoring it from a saved window configuration. The collector is supposed to remove all dead windows whose configurations have died in its finalization phase. For testing purposes, this hash table is accessible from Lisp via the variable 'window-dead-windows-table'. In earlier versions of Emacs, window configurations were mostly used to handle window excursions and were consequently rather short-lived. Your tab bar code has changed that. Hence we should try to avoid any leaks introduced by long-lived configurations. The hash table based code should also work for the incremental (MPS based) collector once it is capable of handling weak hash tables correctly. The present approach to emulate the above mentioned separate mark steps in Elisp is completely inadequate in this regard since it cannot identify dead windows. Please test the patch intensively and report any problems you see. I used the forms below to test the various features of the patch. martin (display-buffer (get-buffer-create "*foo*")) (display-buffer (get-buffer-create "*bar*")) (setq kill-buffer-quit-windows t) (kill-buffer "*bar*") (kill-buffer "*foo*") (setq foo (current-window-configuration)) (set-window-configuration foo) (setq foo nil) ; To reclaim all dead windows of foo (garbage-collect) (hash-table-count window-dead-windows-table) [-- Attachment #2: windows-and-buffers.diff --] [-- Type: text/x-patch, Size: 23857 bytes --] diff --git a/lisp/window.el b/lisp/window.el index 9fc2e5d65e8..0a82aa9989d 100644 --- a/lisp/window.el +++ b/lisp/window.el @@ -4542,14 +4542,18 @@ record-window-buffer (push-window-buffer-onto-prev window) (run-hooks 'buffer-list-update-hook)))) +;; A version in C that handles dead windows (that presumably are part of +;; a saved window configuration) lives in window.c and is called +;; window_discard_buffer_from_dead_window. We could call that from here +;; but it seems more practical to handle live windows entirely in Elisp. (defun unrecord-window-buffer (&optional 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 must be a live window and defaults to the selected one. BUFFER +must be a live buffer and defaults to the buffer of WINDOW (although +that default hardly makes any sense). -Make BUFFER disappear from all variables mentioned by the object of -WINDOW. This includes the buffers previously shwon in WINDOW as well as +Make BUFFER disappear from most components specified by the object of +WINDOW. This includes the buffers previously shown 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)) @@ -4562,7 +4566,13 @@ unrecord-window-buffer window (assq-delete-all buffer (window-prev-buffers window))) (set-window-next-buffers window (delq buffer (window-next-buffers window))) - + ;; If the fourth elements of the 'quit-restore' or + ;; 'quit-restore-prev' parameters equal BUFFER, these parameters + ;; become useless - in 'quit-restore-window' the fourth element + ;; must equal the buffer of WINDOW in order to use that parameter. + ;; If BUFFER is mentioned in the second element of the parameter, + ;; 'quit-restore-window' cannot possibly show BUFFER instead; so + ;; this parameter becomes useless too. (when (or (eq buffer (nth 3 quit-restore-prev)) (and (listp (setq quad (nth 1 quit-restore-prev))) (eq (car quad) buffer))) @@ -4572,7 +4582,8 @@ unrecord-window-buffer (and (listp (setq quad (nth 1 quit-restore))) (eq (car quad) buffer))) (set-window-parameter - window 'quit-restore (window-parameter window 'quit-restore-prev)))))) + window 'quit-restore (window-parameter window 'quit-restore-prev)) + (set-window-parameter window 'quit-restore-prev nil))))) (defun set-window-buffer-start-and-point (window buffer &optional start point) "Set WINDOW's buffer to BUFFER. @@ -5091,6 +5102,18 @@ previous-buffer (not (or executing-kbd-macro noninteractive))) (user-error "No previous buffer")))))) +(defcustom kill-buffer-quit-windows nil + "Non-nil means killing buffers shall quit windows. +If this is nil, killing a buffer may delete dedicated windows only. If +this is non-nil, `kill-buffer' (and `replace-buffer-in-windows' in +consequence) have `quit-restore-window' deal with any window showing the +buffer to be killed which may delete the window if it's not dedicated to +its buffer. Also, `delete-windows-on' will use `quit-restore-window' as +fallback when a window cannot be deleted otherwise." + :type 'boolean + :version "31.1" + :group 'windows) + (defun delete-windows-on (&optional buffer-or-name frame) "Delete all windows showing BUFFER-OR-NAME. BUFFER-OR-NAME may be a buffer or the name of an existing buffer @@ -5128,8 +5151,10 @@ delete-windows-on 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. +any such case, call either `quit-restore-window' (provided +`kill-buffer-quit-windows' is non-nil) or `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 @@ -5166,9 +5191,18 @@ delete-windows-on ((eq deletable t) ;; Delete window. (delete-window window)) + (kill-buffer-quit-windows + (quit-restore-window window 'bury) + (when (window-live-p window) + ;; Unrecord BUFFER in this window. + (unrecord-window-buffer window buffer))) (t - (when (eq (window-buffer window) buffer) - (quit-restore-window window 'bury)) + ;; 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 (window-live-p window) ;; Unrecord BUFFER in this window. (unrecord-window-buffer window buffer))))) @@ -5177,29 +5211,46 @@ delete-windows-on (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. 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 -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 +BUFFER-OR-NAME may be a buffer or the name of an existing buffer and +defaults to the current buffer. Minibuffer windows are not considered. + +If the option `kill-buffer-quit-windows' is nil, behave as follows: With +the exception of side windows, when a window showing BUFFER-OR-NAME is +dedicated, delete that window. If that window is the only window on its +frame, delete its frame when there are other frames left. In any other +case, call `switch-to-prev-buffer' to display some other buffer in that window. -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." +If `kill-buffer-quit-windows' is non-nil, call `quit-restore-window' for +any window showing BUFFER-OR-NAME with the argument BURY-OR-KILL set to +`killing' to avoid that the latter kills the buffer prematurely. + +In either case, remove the buffer denoted by BUFFER-OR-NAME from the +lists of previous and next buffers of all windows and remove any +`quit-restore' or `quit-restore-prev' parameters mentioning it. + +This function is called by `kill-buffer' which kills the buffer +specified by `buffer-or-name' afterwards. It never kills a buffer by +itself." (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. + ;; Scan all windows. We have to unrecord BUFFER-OR-NAME in those + ;; not showing it. (dolist (window (window-list-1 nil nil t)) (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))))) + (if kill-buffer-quit-windows + (quit-restore-window window 'killing) + (let ((dedicated-side (eq (window-dedicated-p window) 'side))) + (when (or dedicated-side (not (window--delete window t 'kill))) + ;; 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)))))) + + (when (window-live-p window) + ;; 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." @@ -5249,7 +5300,14 @@ quit-restore-window most reliable remedy to not have `switch-to-prev-buffer' switch to this buffer again without killing the buffer. -`kill' means to kill WINDOW's buffer." +`kill' means to kill WINDOW's buffer. + +`killing' is like `kill' but means that WINDOW's buffer will get killed +elsewhere. This value is used by `replace-buffer-in-windows' and +`quit-windows-on'. + +`burying' is like `bury' but means that WINDOW's buffer will get buried +elsewhere. This value is used by `quit-windows-on'." (setq window (window-normalize-window window t)) (let* ((buffer (window-buffer window)) (quit-restore (window-parameter window 'quit-restore)) @@ -5265,32 +5323,30 @@ quit-restore-window (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))) + (window--delete + window 'dedicated (memq bury-or-kill '(kill killing)))) ;; If the previously selected window is still alive, select it. (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)) ((and (not prev-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))) + (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))) + (window--delete + window nil (memq bury-or-kill '(kill killing)))) ;; If the previously selected window is still alive, select it. (window--quit-restore-select-window quit-restore-2)) ((or (and (listp (setq quad (nth 1 quit-restore-prev))) @@ -5328,7 +5384,7 @@ quit-restore-window ;; 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 + (when (memq bury-or-kill '(bury burying kill killing)) ;; Remove buffer from WINDOW's previous and next buffers. (set-window-prev-buffers window (assq-delete-all buffer (window-prev-buffers window))) @@ -5359,13 +5415,14 @@ 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))))) + (window--delete + window nil (memq bury-or-kill '(kill killing)))))) ;; Deal with the buffer. (cond ((not (buffer-live-p buffer))) ((eq bury-or-kill 'kill) (kill-buffer buffer)) - (bury-or-kill + ((eq bury-or-kill 'bury) (bury-buffer-internal buffer))))) (defun quit-window (&optional kill window) @@ -5406,11 +5463,18 @@ quit-windows-on (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)) + (quit-restore-window + window (if kill 'killing 'burying))) (when (window-live-p window) ;; Unrecord BUFFER in this window. - (unrecord-window-buffer window buffer))))) + (unrecord-window-buffer window buffer))) + + ;; Deal with BUFFER-OR-NAME. + (cond + ((not (buffer-live-p buffer))) + (kill (kill-buffer buffer)) + (t (bury-buffer-internal buffer))))) \f (defun window--combination-resizable (parent &optional horizontal) "Return number of pixels recoverable from height of window PARENT. @@ -6723,9 +6787,11 @@ display-buffer-record-window 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." +update a `quit-restore-prev' parameter for this window. This allows for +quitting WINDOW in a similar fashion but also keeps the very first +`quit-restore' parameter stored for this window around. Consequently, +WINDOW (or its frame) can be eventually deleted by `quit-restore-widow' +if that parameter's fourth element equals WINDOW's buffer." (cond ((eq type 'reuse) (if (eq (window-buffer window) buffer) diff --git a/src/alloc.c b/src/alloc.c index 666f77bfce1..b955651f5c0 100644 --- a/src/alloc.c +++ b/src/alloc.c @@ -6998,33 +6998,6 @@ mark_face_cache (struct face_cache *c) } } -/* Remove killed buffers or items whose car is a killed buffer from - LIST, and mark other items. Return changed LIST, which is marked. */ - -static Lisp_Object -mark_discard_killed_buffers (Lisp_Object list) -{ - Lisp_Object tail, *prev = &list; - - for (tail = list; CONSP (tail) && !cons_marked_p (XCONS (tail)); - tail = XCDR (tail)) - { - Lisp_Object tem = XCAR (tail); - if (CONSP (tem)) - tem = XCAR (tem); - if (BUFFERP (tem) && !BUFFER_LIVE_P (XBUFFER (tem))) - *prev = XCDR (tail); - else - { - set_cons_marked (XCONS (tail)); - mark_object (XCAR (tail)); - prev = xcdr_addr (tail); - } - } - mark_object (tail); - return list; -} - static void mark_frame (struct Lisp_Vector *ptr) { @@ -7079,15 +7052,6 @@ mark_window (struct Lisp_Vector *ptr) mark_glyph_matrix (w->current_matrix); mark_glyph_matrix (w->desired_matrix); } - - /* Filter out killed buffers from both buffer lists - in attempt to help GC to reclaim killed buffers faster. - We can do it elsewhere for live windows, but this is the - best place to do it for dead windows. */ - wset_prev_buffers - (w, mark_discard_killed_buffers (w->prev_buffers)); - wset_next_buffers - (w, mark_discard_killed_buffers (w->next_buffers)); } /* Entry of the mark stack. */ diff --git a/src/buffer.c b/src/buffer.c index 744b0ef5548..6ec40aff646 100644 --- a/src/buffer.c +++ b/src/buffer.c @@ -2012,6 +2012,13 @@ DEFUN ("kill-buffer", Fkill_buffer, Skill_buffer, 0, 1, "bKill buffer: ", buffer (bug#10114). */ replace_buffer_in_windows (buffer); + /* For dead windows that have not been collected yet, remove this + buffer from those windows' lists of previously and next shown + buffers and remove any 'quit-restore' or 'quit-restore-prev' + parameters mentioning the buffer. */ + if (XFIXNUM (BVAR (b, display_count)) > 0) + window_discard_buffer_from_dead_windows (buffer); + /* Exit if replacing the buffer in windows has killed our buffer. */ if (!BUFFER_LIVE_P (b)) return Qt; diff --git a/src/window.c b/src/window.c index ff28bac5306..dd0abe84fdf 100644 --- a/src/window.c +++ b/src/window.c @@ -3277,6 +3277,90 @@ window_pixel_to_total (Lisp_Object frame, Lisp_Object horizontal) } +/* Remove first occurrence of element whose car is BUFFER from ALIST. + Return changed ALIST. */ +static Lisp_Object +window_discard_buffer_from_alist (Lisp_Object buffer, Lisp_Object alist) +{ + Lisp_Object tail, *prev = &alist; + + for (tail = alist; CONSP (tail); tail = XCDR (tail)) + { + Lisp_Object tem = XCAR (tail); + + tem = XCAR (tem); + + if (EQ (tem, buffer)) + { + *prev = XCDR (tail); + break; + } + else + prev = xcdr_addr (tail); + } + + return alist; +} + +/* Remove first occurrence of BUFFER from LIST. Return changed + LIST. */ +static Lisp_Object +window_discard_buffer_from_list (Lisp_Object buffer, Lisp_Object list) +{ + Lisp_Object tail, *prev = &list; + + for (tail = list; CONSP (tail); tail = XCDR (tail)) + { + if (EQ (XCAR (tail), buffer)) + { + *prev = XCDR (tail); + break; + } + else + prev = xcdr_addr (tail); + } + + return list; +} + +static void +window_discard_buffer_from_dead_window (Lisp_Object buffer, Lisp_Object window) +{ + struct window *w = XWINDOW (window); + Lisp_Object quit_restore = window_parameter (w, Qquit_restore); + Lisp_Object quit_restore_prev = window_parameter (w, Qquit_restore_prev); + Lisp_Object quad; + + wset_prev_buffers + (w, window_discard_buffer_from_alist (buffer, w->prev_buffers)); + wset_next_buffers + (w, window_discard_buffer_from_list (buffer, w->next_buffers)); + + if (EQ (buffer, Fnth (make_fixnum (3), quit_restore_prev)) + || (CONSP (quad = Fcar (Fcdr (quit_restore_prev))) + && EQ (Fcar (quad), buffer))) + Fset_window_parameter (window, Qquit_restore_prev, Qnil); + + if (EQ (buffer, Fnth (make_fixnum (3), quit_restore)) + || (CONSP (quad = Fcar (Fcdr (quit_restore))) + && EQ (Fcar (quad), buffer))) + { + Fset_window_parameter (window, Qquit_restore, + window_parameter (w, Qquit_restore_prev)); + Fset_window_parameter (window, Qquit_restore_prev, Qnil); + } +} + +void +window_discard_buffer_from_dead_windows (Lisp_Object buffer) +{ + struct Lisp_Hash_Table *h = XHASH_TABLE (window_dead_windows_table); + + DOHASH (h, k, v) + window_discard_buffer_from_dead_window (buffer, v); +} + + DEFUN ("delete-other-windows-internal", Fdelete_other_windows_internal, Sdelete_other_windows_internal, 0, 2, "", doc: /* Make WINDOW fill its frame. @@ -4402,6 +4486,10 @@ make_parent_window (Lisp_Object window, bool horflag) wset_buffer (p, Qnil); wset_combination (p, horflag, window); wset_combination_limit (p, Qnil); + /* Reset any previous and next buffers of p which have been installed + by the memcpy above. */ + wset_prev_buffers (p, Qnil); + wset_next_buffers (p, Qnil); wset_window_parameters (p, Qnil); } @@ -4426,10 +4514,6 @@ make_window (void) wset_vertical_scroll_bar_type (w, Qt); wset_horizontal_scroll_bar_type (w, Qt); wset_cursor_type (w, Qt); - /* These Lisp fields are marked specially so they're not set to nil by - allocate_window. */ - wset_prev_buffers (w, Qnil); - wset_next_buffers (w, Qnil); /* Initialize non-Lisp data. Note that allocate_window zeroes out all non-Lisp data, so do it only for slots which should not be zero. */ @@ -5252,6 +5336,11 @@ DEFUN ("delete-window-internal", Fdelete_window_internal, Sdelete_window_interna unchain_marker (XMARKER (w->old_pointm)); unchain_marker (XMARKER (w->start)); wset_buffer (w, Qnil); + /* Add WINDOW to table of dead windows so when killing a buffer + WINDOW mentions, all references to that buffer can be removed + and the buffer be collected. */ + Fputhash (make_fixnum (w->sequence_number), + window, window_dead_windows_table); } if (NILP (s->prev) && NILP (s->next)) @@ -7356,6 +7445,10 @@ DEFUN ("set-window-configuration", Fset_window_configuration, } } + /* Remove window from the table of dead windows. */ + Fremhash (make_fixnum (w->sequence_number), + window_dead_windows_table); + if ((NILP (dont_set_miniwindow) || !MINI_WINDOW_P (w)) && BUFFERP (p->buffer) && BUFFER_LIVE_P (XBUFFER (p->buffer))) /* If saved buffer is alive, install it, unless it's a @@ -7585,6 +7678,11 @@ delete_all_child_windows (Lisp_Object window) possible resurrection in Fset_window_configuration. */ wset_combination_limit (w, w->contents); wset_buffer (w, Qnil); + /* Add WINDOW to table of dead windows so when killing a buffer + WINDOW mentions, all references to that buffer can be removed + and the buffer be collected. */ + Fputhash (make_fixnum (w->sequence_number), + window, window_dead_windows_table); } Vwindow_list = Qnil; @@ -8594,6 +8692,8 @@ syms_of_window (void) DEFSYM (Qconfiguration, "configuration"); DEFSYM (Qdelete, "delete"); DEFSYM (Qdedicated, "dedicated"); + DEFSYM (Qquit_restore, "quit-restore"); + DEFSYM (Qquit_restore_prev, "quit-restore-prev"); DEFVAR_LISP ("temp-buffer-show-function", Vtemp_buffer_show_function, doc: /* Non-nil means call as function to display a help buffer. @@ -8917,6 +9017,17 @@ syms_of_window (void) displayed after a scrolling operation to be somewhat inaccurate. */); fast_but_imprecise_scrolling = false; + DEFVAR_LISP ("window-dead-windows-table", window_dead_windows_table, + doc: /* Hash table of dead windows. +Each entry in this table maps a window number to a window object. +Entries are added by `delete-window-internal' and are removed by the +garbage collector. + +This table is maintained by code in window.c and is made visible in +Elisp for testing purposes only. */); + window_dead_windows_table + = CALLN (Fmake_hash_table, QCweakness, Qt); + defsubr (&Sselected_window); defsubr (&Sold_selected_window); defsubr (&Sminibuffer_window); diff --git a/src/window.h b/src/window.h index 86932181252..335e0a3453e 100644 --- a/src/window.h +++ b/src/window.h @@ -142,6 +142,12 @@ #define WINDOW_H_INCLUDED as well. */ Lisp_Object contents; + /* A list of <buffer, window-start, window-point> triples listing + buffers previously shown in this window. */ + Lisp_Object prev_buffers; + /* List of buffers re-shown in this window. */ + Lisp_Object next_buffers; + /* The old buffer of this window, set to this window's buffer by run_window_change_functions every time it sees this window. Unused for internal windows. */ @@ -218,14 +224,6 @@ #define WINDOW_H_INCLUDED struct glyph_matrix *current_matrix; struct glyph_matrix *desired_matrix; - /* The two Lisp_Object fields below are marked in a special way, - which is why they're placed after `current_matrix'. */ - /* A list of <buffer, window-start, window-point> triples listing - buffers previously shown in this window. */ - Lisp_Object prev_buffers; - /* List of buffers re-shown in this window. */ - Lisp_Object next_buffers; - /* Number saying how recently window was selected. */ EMACS_INT use_time; @@ -1228,6 +1226,7 @@ #define CHECK_LIVE_WINDOW(WINDOW) \ extern void wset_buffer (struct window *, Lisp_Object); extern bool window_outdated (struct window *); extern ptrdiff_t window_point (struct window *w); +extern void window_discard_buffer_from_dead_windows (Lisp_Object); extern void init_window_once (void); extern void init_window (void); extern void syms_of_window (void); ^ permalink raw reply related [flat|nested] 45+ messages in thread
* bug#59862: quit-restore per window buffer 2024-07-08 16:49 ` martin rudalics via Bug reports for GNU Emacs, the Swiss army knife of text editors @ 2024-07-09 6:58 ` Juri Linkov 2024-07-09 8:52 ` martin rudalics via Bug reports for GNU Emacs, the Swiss army knife of text editors 0 siblings, 1 reply; 45+ messages in thread From: Juri Linkov @ 2024-07-09 6:58 UTC (permalink / raw) To: martin rudalics; +Cc: 59862 > It took me some time to rewrite the code for this since I found a couple > of leaks in the window handling code which I tried to plug now. In a > nutshell, the new patch is supposed to fix the following issues. > > - I introduced a new variable called 'kill-buffer-quit-windows'. If > non-nil, it makes killing a buffer call 'quit-restore-window' for each > window showing the buffer thus implementing the behavior you want. If > nil, killing a buffer works as before apart from the fixes I added > below. Thanks, I hope it will help to replace such configurations: (defun quit-window-kill-buffer () "Quit WINDOW and kill its buffer." (interactive) (quit-window 1)) (define-key special-mode-map "q" 'quit-window-kill-buffer) (define-key image-mode-map "q" 'quit-window-kill-buffer) (define-key global-map "\C-q" 'quit-window-kill-buffer) (define-key dired-mode-map "q" 'quit-window-kill-buffer) (define-key archive-mode-map "q" 'quit-window-kill-buffer) (define-key tar-mode-map "q" 'quit-window-kill-buffer) ... I suppose it will affect only interactive uses of 'kill-buffer'. > In earlier versions of Emacs, window configurations were mostly used to > handle window excursions and were consequently rather short-lived. Your > tab bar code has changed that. Hence we should try to avoid any leaks > introduced by long-lived configurations. Indeed now window configurations are long-lived, so tab-bar uses (split-window) followed by (delete-window) to create a new window different from the window saved in a window configuration. > Please test the patch intensively and report any problems you see. I > used the forms below to test the various features of the patch. I have used your previous patch for a long time, and it worked perfectly. I hope your new patch will be as good as the old one. PS: I noticed one difference between handling of frames and tabs: C-x 5 5 ;; other-frame-prefix C-h i ;; info C-h e ;; view-echo-area-messages q ;; quit-window This deletes the Info window, and doesn't delete the frame. Everything is correct. C-x t t ;; other-tab-prefix C-h i ;; info C-h e ;; view-echo-area-messages q ;; quit-window This doesn't close the tab (correct), but doesn't delete the Info window. This differs from the frame case that keeps only the Messages window on the frame. ^ permalink raw reply [flat|nested] 45+ messages in thread
* bug#59862: quit-restore per window buffer 2024-07-09 6:58 ` Juri Linkov @ 2024-07-09 8:52 ` martin rudalics via Bug reports for GNU Emacs, the Swiss army knife of text editors 2024-07-10 6:50 ` Juri Linkov 0 siblings, 1 reply; 45+ messages in thread From: martin rudalics via Bug reports for GNU Emacs, the Swiss army knife of text editors @ 2024-07-09 8:52 UTC (permalink / raw) To: Juri Linkov; +Cc: 59862 > I suppose it will affect only interactive uses of 'kill-buffer'. Why? It affects all uses of 'kill-buffer'. > PS: I noticed one difference between handling of frames and tabs: > > C-x 5 5 ;; other-frame-prefix > C-h i ;; info > C-h e ;; view-echo-area-messages > q ;; quit-window > This deletes the Info window, and doesn't delete the frame. > Everything is correct. > > C-x t t ;; other-tab-prefix > C-h i ;; info > C-h e ;; view-echo-area-messages > q ;; quit-window > This doesn't close the tab (correct), > but doesn't delete the Info window. > This differs from the frame case that keeps > only the Messages window on the frame. Suppose you do C-h i ;; info C-h e ;; view-echo-area-messages q ;; quit-window Then the difference is obvious. The C-h i reuses the one existing window showing *scratch*. 'display-buffer-record-window' cannot tell this case apart from the one where a user does want to show *scratch* again in that window after being done with *info*. If we wanted a behavior that deleted the *info* window in this case (it could do that only if there is another window on that frame), the caller could add an alist entry, say 'delete-reused-window-if-possible', and add something like (when (and (eq type 'reuse) (assq 'delete-reused-window-if-possible alist)) (setq type 'window)) immediately before the (display-buffer-record-window type window buffer) call in 'window--display-buffer'. But you pass 'tab' as TYPE argument to 'window--display-buffer' and that would have to be handled in a maybe similar but still different way. martin ^ permalink raw reply [flat|nested] 45+ messages in thread
* bug#59862: quit-restore per window buffer 2024-07-09 8:52 ` martin rudalics via Bug reports for GNU Emacs, the Swiss army knife of text editors @ 2024-07-10 6:50 ` Juri Linkov 2024-07-10 9:16 ` martin rudalics via Bug reports for GNU Emacs, the Swiss army knife of text editors 0 siblings, 1 reply; 45+ messages in thread From: Juri Linkov @ 2024-07-10 6:50 UTC (permalink / raw) To: martin rudalics; +Cc: 59862 >> I suppose it will affect only interactive uses of 'kill-buffer'. > > Why? It affects all uses of 'kill-buffer'. I'm not sure about this, I just had a feeling that killing a temporary buffer should not affect its window. But a test case is needed to see if this is a real problem. >> PS: I noticed one difference between handling of frames and tabs: >> >> C-x 5 5 ;; other-frame-prefix >> C-h i ;; info >> C-h e ;; view-echo-area-messages >> q ;; quit-window >> This deletes the Info window, and doesn't delete the frame. >> Everything is correct. >> >> C-x t t ;; other-tab-prefix >> C-h i ;; info >> C-h e ;; view-echo-area-messages >> q ;; quit-window >> This doesn't close the tab (correct), >> but doesn't delete the Info window. >> This differs from the frame case that keeps >> only the Messages window on the frame. > > Suppose you do > > C-h i ;; info > C-h e ;; view-echo-area-messages > q ;; quit-window > > Then the difference is obvious. The C-h i reuses the one existing > window showing *scratch*. 'display-buffer-record-window' cannot tell > this case apart from the one where a user does want to show *scratch* > again in that window after being done with *info*. But there is no *scratch* in the list of previous buffers for 'C-x t t C-h i' (that can be confirmed when tab-line is enabled), so the user won't expect to show *scratch*. And indeed the frame case doesn't show *scratch*. I hoped that it would be possible for the tab case to do the same, but I don't know how the frame case is handled to do the right thing. ^ permalink raw reply [flat|nested] 45+ messages in thread
* bug#59862: quit-restore per window buffer 2024-07-10 6:50 ` Juri Linkov @ 2024-07-10 9:16 ` martin rudalics via Bug reports for GNU Emacs, the Swiss army knife of text editors 2024-07-11 6:47 ` Juri Linkov 0 siblings, 1 reply; 45+ messages in thread From: martin rudalics via Bug reports for GNU Emacs, the Swiss army knife of text editors @ 2024-07-10 9:16 UTC (permalink / raw) To: Juri Linkov; +Cc: 59862 > But there is no *scratch* in the list of previous buffers for 'C-x t t C-h i' > (that can be confirmed when tab-line is enabled), so the user won't expect > to show *scratch*. And indeed the frame case doesn't show *scratch*. 'window--display-buffer' resets that window's previous buffers here (when (memq type '(window frame tab)) (set-window-prev-buffers window nil)) In the "window" and "frame" case these are actually not needed because when making a new window (on the same or a new frame), that window's previous buffers are automatically set to nil - by make_window in the release version and by the allocation routine with my patch. I reset them just to make sure. When you show *info* in an already existing window, that window's previous buffers are not reset. So the presence of "tab" in the form above is not entirely correct in this regard. > I hoped that it would be possible for the tab case to do the same, > but I don't know how the frame case is handled to do the right thing. In 'quit-restore-window' you count the windows with a 'quit-restore' parameter. In the scenario at hand this gives 2 so you don't close the tab. If you told me in detail what this counting is supposed to accomplish, I might be able to help you. martin ^ permalink raw reply [flat|nested] 45+ messages in thread
* bug#59862: quit-restore per window buffer 2024-07-10 9:16 ` martin rudalics via Bug reports for GNU Emacs, the Swiss army knife of text editors @ 2024-07-11 6:47 ` Juri Linkov 2024-07-11 8:36 ` martin rudalics via Bug reports for GNU Emacs, the Swiss army knife of text editors 0 siblings, 1 reply; 45+ messages in thread From: Juri Linkov @ 2024-07-11 6:47 UTC (permalink / raw) To: martin rudalics; +Cc: 59862 >> But there is no *scratch* in the list of previous buffers for 'C-x t t C-h i' >> (that can be confirmed when tab-line is enabled), so the user won't expect >> to show *scratch*. And indeed the frame case doesn't show *scratch*. > > 'window--display-buffer' resets that window's previous buffers here > > (when (memq type '(window frame tab)) > (set-window-prev-buffers window nil)) > > In the "window" and "frame" case these are actually not needed because > when making a new window (on the same or a new frame), that window's > previous buffers are automatically set to nil - by make_window in the > release version and by the allocation routine with my patch. I reset > them just to make sure. When you show *info* in an already existing > window, that window's previous buffers are not reset. So the presence > of "tab" in the form above is not entirely correct in this regard. I checked that prev-buffers is nil in both frame and tab cases, so everything is correct here. >> I hoped that it would be possible for the tab case to do the same, >> but I don't know how the frame case is handled to do the right thing. > > In 'quit-restore-window' you count the windows with a 'quit-restore' > parameter. In the scenario at hand this gives 2 so you don't close the > tab. If you told me in detail what this counting is supposed to > accomplish, I might be able to help you. 'seq-count' with (window-parameter w 'quit-restore) is intended to be equivalent to (frame-root-window window) in the frame case, and even better. So the problem is not here. The difference between frame and tab case is that the frame branch calls 'window--delete' that decides whether to delete the frame, or just delete the window. But in the tab case it either closes the tab, or does nothing. So in the tab case it should call 'window--delete' as well, that will decide whether to close the tab, or just delete the window. Then 'window--delete' should do something like this: (unless (and dedicated-only (not (window-dedicated-p window))) (let ((deletable (window-deletable-p window))) (cond + ((eq deletable 'tab) + (tab-bar-close-tab) + 'tab) ((eq deletable 'frame) (let ((frame (window-frame window))) (cond Then maybe 'seq-count' with (window-parameter w 'quit-restore) should be moved to 'window-deletable-p'. ^ permalink raw reply [flat|nested] 45+ messages in thread
* bug#59862: quit-restore per window buffer 2024-07-11 6:47 ` Juri Linkov @ 2024-07-11 8:36 ` martin rudalics via Bug reports for GNU Emacs, the Swiss army knife of text editors 2024-07-12 6:54 ` Juri Linkov 0 siblings, 1 reply; 45+ messages in thread From: martin rudalics via Bug reports for GNU Emacs, the Swiss army knife of text editors @ 2024-07-11 8:36 UTC (permalink / raw) To: Juri Linkov; +Cc: 59862 > I checked that prev-buffers is nil in both frame and tab cases, > so everything is correct here. It's not correct in the sense that in the tab case the window showing *info* had a previous buffer - *scratch*. Think of users working on some buffer who want to consult Info on something in that buffer with the intention to turn back to that buffer as soon as they are done. While consulting *info* they hit C-h e. Now quitting *info* would delete the window showing *info* and leave them with a window showing *Messages* instead. Is that really the behavior you have in mind? How would a user go back to the original buffer? > The difference between frame and tab case is that the frame branch > calls 'window--delete' that decides whether to delete the frame, > or just delete the window. But in the tab case it either > closes the tab, or does nothing. So in the tab case > it should call 'window--delete' as well, that will decide > whether to close the tab, or just delete the window. > > Then 'window--delete' should do something like this: > > (unless (and dedicated-only (not (window-dedicated-p window))) > (let ((deletable (window-deletable-p window))) > (cond > + ((eq deletable 'tab) > + (tab-bar-close-tab) > + 'tab) > ((eq deletable 'frame) > (let ((frame (window-frame window))) > (cond > > Then maybe 'seq-count' with (window-parameter w 'quit-restore) > should be moved to 'window-deletable-p'. IIUC when 'display-buffer-record-window' installs a 'quit-restore' parameter of the 'tab' type, it assumes full responsibility for all windows on that frame as long as the corresponding tab has not been closed. This means that the tab code has to assure a coherent state of the frame's windows after the tab has been closed. If you can guarantee that, feel free to move that check wherever you want. martin ^ permalink raw reply [flat|nested] 45+ messages in thread
* bug#59862: quit-restore per window buffer 2024-07-11 8:36 ` martin rudalics via Bug reports for GNU Emacs, the Swiss army knife of text editors @ 2024-07-12 6:54 ` Juri Linkov 2024-07-12 8:20 ` martin rudalics via Bug reports for GNU Emacs, the Swiss army knife of text editors 0 siblings, 1 reply; 45+ messages in thread From: Juri Linkov @ 2024-07-12 6:54 UTC (permalink / raw) To: martin rudalics; +Cc: 59862 >> I checked that prev-buffers is nil in both frame and tab cases, >> so everything is correct here. > > It's not correct in the sense that in the tab case the window showing > *info* had a previous buffer - *scratch*. Think of users working on > some buffer who want to consult Info on something in that buffer with > the intention to turn back to that buffer as soon as they are done. > While consulting *info* they hit C-h e. Now quitting *info* would > delete the window showing *info* and leave them with a window showing > *Messages* instead. Is that really the behavior you have in mind? How > would a user go back to the original buffer? The tab case should not differ from the frame case that has no previous buffer. >> The difference between frame and tab case is that the frame branch >> calls 'window--delete' that decides whether to delete the frame, >> or just delete the window. But in the tab case it either >> closes the tab, or does nothing. So in the tab case >> it should call 'window--delete' as well, that will decide >> whether to close the tab, or just delete the window. >> >> Then 'window--delete' should do something like this: >> >> (unless (and dedicated-only (not (window-dedicated-p window))) >> (let ((deletable (window-deletable-p window))) >> (cond >> + ((eq deletable 'tab) >> + (tab-bar-close-tab) >> + 'tab) >> ((eq deletable 'frame) >> (let ((frame (window-frame window))) >> (cond >> >> Then maybe 'seq-count' with (window-parameter w 'quit-restore) >> should be moved to 'window-deletable-p'. > > IIUC when 'display-buffer-record-window' installs a 'quit-restore' > parameter of the 'tab' type, it assumes full responsibility for all > windows on that frame as long as the corresponding tab has not been > closed. This means that the tab code has to assure a coherent state of > the frame's windows after the tab has been closed. If you can guarantee > that, feel free to move that check wherever you want. Ok, I will try to move it. ^ permalink raw reply [flat|nested] 45+ messages in thread
* bug#59862: quit-restore per window buffer 2024-07-12 6:54 ` Juri Linkov @ 2024-07-12 8:20 ` martin rudalics via Bug reports for GNU Emacs, the Swiss army knife of text editors 2024-07-14 7:49 ` Juri Linkov 0 siblings, 1 reply; 45+ messages in thread From: martin rudalics via Bug reports for GNU Emacs, the Swiss army knife of text editors @ 2024-07-12 8:20 UTC (permalink / raw) To: Juri Linkov; +Cc: 59862 > Ok, I will try to move it. We could replace the (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))))) clause with (or (eq (nth 1 quit-restore) 'frame) ;; 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)))) with the motivation that if a window does not have a previous buffer, there is no reason to switch to it. This will keep the frame around so Bu#12764 is not affected and the normal behavior of C-h i followed by C-h e is not affected either unless a user deleted *scratch* in between. Try it and if it works for you I'll add it to the new patch. martin ^ permalink raw reply [flat|nested] 45+ messages in thread
* bug#59862: quit-restore per window buffer 2024-07-12 8:20 ` martin rudalics via Bug reports for GNU Emacs, the Swiss army knife of text editors @ 2024-07-14 7:49 ` Juri Linkov 2024-07-15 7:32 ` martin rudalics via Bug reports for GNU Emacs, the Swiss army knife of text editors 2024-07-30 8:20 ` martin rudalics via Bug reports for GNU Emacs, the Swiss army knife of text editors 0 siblings, 2 replies; 45+ messages in thread From: Juri Linkov @ 2024-07-14 7:49 UTC (permalink / raw) To: martin rudalics; +Cc: 59862 >> Ok, I will try to move it. > > We could replace the > > (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))))) > > clause with > > (or (eq (nth 1 quit-restore) 'frame) > ;; 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)))) > > with the motivation that if a window does not have a previous buffer, > there is no reason to switch to it. This will keep the frame around so > Bu#12764 is not affected and the normal behavior of C-h i followed by > C-h e is not affected either unless a user deleted *scratch* in between. Sorry, I know nothing about the frame case. > Try it and if it works for you I'll add it to the new patch. OTOH, I tried to add the tab case handling to the same places where the frame case is handled, and everything works nicely with this patch applied over your previous patches: diff --git a/lisp/window.el b/lisp/window.el index 58120c919c7..82efb3c40ce 100644 --- a/lisp/window.el +++ b/lisp/window.el @@ -4120,6 +4120,11 @@ window-deletable-p (let ((frame (window-frame window))) (cond + ((and (eq (nth 1 (window-parameter window 'quit-restore)) 'tab) + (< (seq-count (lambda (w) (window-parameter w 'quit-restore)) + (window-list-1 nil 'nomini)) + 2)) + 'tab) ((frame-root-window-p window) ;; WINDOW's frame can be deleted only if there are other frames ;; on the same terminal, and it does not contain the active @@ -4990,6 +4995,9 @@ window--delete (unless (and dedicated-only (not (window-dedicated-p window))) (let ((deletable (window-deletable-p window))) (cond + ((eq deletable 'tab) + (tab-bar-close-tab) + 'tab) ((eq deletable 'frame) (let ((frame (window-frame window))) (cond @@ -5303,10 +5311,8 @@ quit-restore-window ((and (not prev-buffer) (eq (nth 1 quit-restore) 'tab) (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) + (window--delete + window nil (memq bury-or-kill '(kill killing)))) ;; If the previously selected window is still alive, select it. (window--quit-restore-select-window quit-restore-2)) ((and (not prev-buffer) ^ permalink raw reply related [flat|nested] 45+ messages in thread
* bug#59862: quit-restore per window buffer 2024-07-14 7:49 ` Juri Linkov @ 2024-07-15 7:32 ` martin rudalics via Bug reports for GNU Emacs, the Swiss army knife of text editors 2024-07-16 6:09 ` Björn Bidar via Bug reports for GNU Emacs, the Swiss army knife of text editors ` (2 more replies) 2024-07-30 8:20 ` martin rudalics via Bug reports for GNU Emacs, the Swiss army knife of text editors 1 sibling, 3 replies; 45+ messages in thread From: martin rudalics via Bug reports for GNU Emacs, the Swiss army knife of text editors @ 2024-07-15 7:32 UTC (permalink / raw) To: Juri Linkov; +Cc: 59862 >> This will keep the frame around so >> Bu#12764 is not affected and the normal behavior of C-h i followed by >> C-h e is not affected either unless a user deleted *scratch* in between. > > Sorry, I know nothing about the frame case. When creating a frame as part of 'display-buffer', quitting the sole window of that frame should delete the frame. When creating a frame separately and calling 'display-buffer' to show a buffer in its only window, quitting the window should not delete the frame even if that window has no previous buffer. See Bug#12764 for the details. > OTOH, I tried to add the tab case handling to the same places > where the frame case is handled, and everything works nicely > with this patch applied over your previous patches: If nobody objects I'll install my patch next week and you can install your changes on top of it. martin ^ permalink raw reply [flat|nested] 45+ messages in thread
* bug#59862: quit-restore per window buffer 2024-07-15 7:32 ` martin rudalics via Bug reports for GNU Emacs, the Swiss army knife of text editors @ 2024-07-16 6:09 ` Björn Bidar via Bug reports for GNU Emacs, the Swiss army knife of text editors 2024-07-16 6:09 ` Björn Bidar via Bug reports for GNU Emacs, the Swiss army knife of text editors [not found] ` <87frs9kgve.fsf@> 2 siblings, 0 replies; 45+ messages in thread From: Björn Bidar via Bug reports for GNU Emacs, the Swiss army knife of text editors @ 2024-07-16 6:09 UTC (permalink / raw) To: 59862; +Cc: rudalics, juri martin rudalics via "Bug reports for GNU Emacs, the Swiss army knife of text editors" <bug-gnu-emacs@gnu.org> writes: >>> This will keep the frame around so >>> Bu#12764 is not affected and the normal behavior of C-h i followed by >>> C-h e is not affected either unless a user deleted *scratch* in between. >> >> Sorry, I know nothing about the frame case. > > When creating a frame as part of 'display-buffer', quitting the sole > window of that frame should delete the frame. When creating a frame > separately and calling 'display-buffer' to show a buffer in its only > window, quitting the window should not delete the frame even if that > window has no previous buffer. See Bug#12764 for the details. What about use cases where the frame was only spawned for that particular window? For a frame focused setup frames mostly contain only one window and don't close when the only window in it is killed leaving a window with the scratch buffer around. ^ permalink raw reply [flat|nested] 45+ messages in thread
* bug#59862: quit-restore per window buffer 2024-07-15 7:32 ` martin rudalics via Bug reports for GNU Emacs, the Swiss army knife of text editors 2024-07-16 6:09 ` Björn Bidar via Bug reports for GNU Emacs, the Swiss army knife of text editors @ 2024-07-16 6:09 ` Björn Bidar via Bug reports for GNU Emacs, the Swiss army knife of text editors [not found] ` <87frs9kgve.fsf@> 2 siblings, 0 replies; 45+ messages in thread From: Björn Bidar via Bug reports for GNU Emacs, the Swiss army knife of text editors @ 2024-07-16 6:09 UTC (permalink / raw) To: 59862; +Cc: rudalics, juri martin rudalics via "Bug reports for GNU Emacs, the Swiss army knife of text editors" <bug-gnu-emacs@gnu.org> writes: >>> This will keep the frame around so >>> Bu#12764 is not affected and the normal behavior of C-h i followed by >>> C-h e is not affected either unless a user deleted *scratch* in between. >> >> Sorry, I know nothing about the frame case. > > When creating a frame as part of 'display-buffer', quitting the sole > window of that frame should delete the frame. When creating a frame > separately and calling 'display-buffer' to show a buffer in its only > window, quitting the window should not delete the frame even if that > window has no previous buffer. See Bug#12764 for the details. What about use cases where the frame was only spawned for that particular window? For a frame focused setup frames mostly contain only one window and don't close when the only window in it is killed leaving a window with the scratch buffer around. ^ permalink raw reply [flat|nested] 45+ messages in thread
[parent not found: <87frs9kgve.fsf@>]
* bug#59862: quit-restore per window buffer [not found] ` <87frs9kgve.fsf@> @ 2024-07-16 8:22 ` martin rudalics via Bug reports for GNU Emacs, the Swiss army knife of text editors 2024-07-16 22:14 ` Björn Bidar via Bug reports for GNU Emacs, the Swiss army knife of text editors ` (2 more replies) 0 siblings, 3 replies; 45+ messages in thread From: martin rudalics via Bug reports for GNU Emacs, the Swiss army knife of text editors @ 2024-07-16 8:22 UTC (permalink / raw) To: bjorn.bidar, 59862; +Cc: juri > What about use cases where the frame was only spawned for that > particular window? > > For a frame focused setup frames mostly contain only one window and > don't close when the only window in it is killed Do you mean "window" or "buffer" here? C-x 0 in a frame containing only one window gets you an "Attempt to delete minibuffer or sole ordinary window" error. I suppose you meant "buffer" here. > leaving a window with > the scratch buffer around. Let's distinguish two different cases: If, after evaluating (custom-set-variables '(display-buffer-alist '(("\\*info\\*" display-buffer-same-window))) '(frame-auto-hide-function 'delete-frame)) you do C-x 5 2 then C-h i and finally q, the second frame will stay around and show *scratch*. Alternatively, you can evaluate (custom-set-variables '(display-buffer-alist '(("\\*info\\*" display-buffer-same-window)))) do C-x 5 2 then C-h i and finally C-u q which also kills the *info* buffer. These are the behaviors needed for handling the Bug#12764 scenario. If instead you evaluate (custom-set-variables '(display-buffer-alist '(("\\*info\\*" display-buffer-pop-up-frame))) '(frame-auto-hide-function 'delete-frame)) and then do C-h i followed by q, the second frame gets deleted. Alternatively you can get the same visual behavior via (custom-set-variables '(display-buffer-alist '(("\\*info\\*" display-buffer-pop-up-frame)))) followed by C-h i and C-u q where the latter also kills the *info* buffer. In the thread of Bug#12764 I argued that deleting the frame would make sense in the first scenario too. But then we would have to look into the history of the last window on that frame to decide whether the buffer it previously showed should be restored or not. Even if there is no previous buffer for that window we should at least optionally allow the frame to stay alive. martin ^ permalink raw reply [flat|nested] 45+ messages in thread
* bug#59862: quit-restore per window buffer 2024-07-16 8:22 ` martin rudalics via Bug reports for GNU Emacs, the Swiss army knife of text editors @ 2024-07-16 22:14 ` Björn Bidar via Bug reports for GNU Emacs, the Swiss army knife of text editors 2024-07-16 22:14 ` Björn Bidar via Bug reports for GNU Emacs, the Swiss army knife of text editors [not found] ` <871q3tc7da.fsf@> 2 siblings, 0 replies; 45+ messages in thread From: Björn Bidar via Bug reports for GNU Emacs, the Swiss army knife of text editors @ 2024-07-16 22:14 UTC (permalink / raw) To: 59862; +Cc: rudalics, juri martin rudalics via "Bug reports for GNU Emacs, the Swiss army knife of text editors" <bug-gnu-emacs@gnu.org> writes: >> What about use cases where the frame was only spawned for that >> particular window? >> >> For a frame focused setup frames mostly contain only one window and >> don't close when the only window in it is killed > > Do you mean "window" or "buffer" here? C-x 0 in a frame containing only > one window gets you an "Attempt to delete minibuffer or sole ordinary > window" error. I suppose you meant "buffer" here. I meant window. Same as you explain further below in your message I call quit-window on a dedicate window the frame also dies with it. >> leaving a window with >> the scratch buffer around. > > Let's distinguish two different cases: If, after evaluating > > (custom-set-variables > '(display-buffer-alist '(("\\*info\\*" display-buffer-same-window))) > '(frame-auto-hide-function 'delete-frame)) > > you do C-x 5 2 then C-h i and finally q, the second frame will stay > around and show *scratch*. Alternatively, you can evaluate > > (custom-set-variables > '(display-buffer-alist '(("\\*info\\*" display-buffer-same-window)))) > > do C-x 5 2 then C-h i and finally C-u q which also kills the *info* > buffer. These are the behaviors needed for handling the Bug#12764 > scenario. > > If instead you evaluate > > (custom-set-variables > '(display-buffer-alist '(("\\*info\\*" display-buffer-pop-up-frame))) > '(frame-auto-hide-function 'delete-frame)) > > and then do C-h i followed by q, the second frame gets deleted. > Alternatively you can get the same visual behavior via > > (custom-set-variables > '(display-buffer-alist '(("\\*info\\*" display-buffer-pop-up-frame)))) > > followed by C-h i and C-u q where the latter also kills the *info* > buffer. > > In the thread of Bug#12764 I argued that deleting the frame would make > sense in the first scenario too. But then we would have to look into > the history of the last window on that frame to decide whether the > buffer it previously showed should be restored or not. Even if there is > no previous buffer for that window we should at least optionally allow > the frame to stay alive. > I very much agree with you on that point. I think if the window inside of a frame didn't contain any other buffer than the initial buffer and a new buffer that uses the same window there could be the argument that the window should die and thus the frame too. ^ permalink raw reply [flat|nested] 45+ messages in thread
* bug#59862: quit-restore per window buffer 2024-07-16 8:22 ` martin rudalics via Bug reports for GNU Emacs, the Swiss army knife of text editors 2024-07-16 22:14 ` Björn Bidar via Bug reports for GNU Emacs, the Swiss army knife of text editors @ 2024-07-16 22:14 ` Björn Bidar via Bug reports for GNU Emacs, the Swiss army knife of text editors [not found] ` <871q3tc7da.fsf@> 2 siblings, 0 replies; 45+ messages in thread From: Björn Bidar via Bug reports for GNU Emacs, the Swiss army knife of text editors @ 2024-07-16 22:14 UTC (permalink / raw) To: 59862; +Cc: rudalics, juri martin rudalics via "Bug reports for GNU Emacs, the Swiss army knife of text editors" <bug-gnu-emacs@gnu.org> writes: >> What about use cases where the frame was only spawned for that >> particular window? >> >> For a frame focused setup frames mostly contain only one window and >> don't close when the only window in it is killed > > Do you mean "window" or "buffer" here? C-x 0 in a frame containing only > one window gets you an "Attempt to delete minibuffer or sole ordinary > window" error. I suppose you meant "buffer" here. I meant window. Same as you explain further below in your message I call quit-window on a dedicate window the frame also dies with it. >> leaving a window with >> the scratch buffer around. > > Let's distinguish two different cases: If, after evaluating > > (custom-set-variables > '(display-buffer-alist '(("\\*info\\*" display-buffer-same-window))) > '(frame-auto-hide-function 'delete-frame)) > > you do C-x 5 2 then C-h i and finally q, the second frame will stay > around and show *scratch*. Alternatively, you can evaluate > > (custom-set-variables > '(display-buffer-alist '(("\\*info\\*" display-buffer-same-window)))) > > do C-x 5 2 then C-h i and finally C-u q which also kills the *info* > buffer. These are the behaviors needed for handling the Bug#12764 > scenario. > > If instead you evaluate > > (custom-set-variables > '(display-buffer-alist '(("\\*info\\*" display-buffer-pop-up-frame))) > '(frame-auto-hide-function 'delete-frame)) > > and then do C-h i followed by q, the second frame gets deleted. > Alternatively you can get the same visual behavior via > > (custom-set-variables > '(display-buffer-alist '(("\\*info\\*" display-buffer-pop-up-frame)))) > > followed by C-h i and C-u q where the latter also kills the *info* > buffer. > > In the thread of Bug#12764 I argued that deleting the frame would make > sense in the first scenario too. But then we would have to look into > the history of the last window on that frame to decide whether the > buffer it previously showed should be restored or not. Even if there is > no previous buffer for that window we should at least optionally allow > the frame to stay alive. > I very much agree with you on that point. I think if the window inside of a frame didn't contain any other buffer than the initial buffer and a new buffer that uses the same window there could be the argument that the window should die and thus the frame too. ^ permalink raw reply [flat|nested] 45+ messages in thread
[parent not found: <871q3tc7da.fsf@>]
* bug#59862: quit-restore per window buffer [not found] ` <871q3tc7da.fsf@> @ 2024-07-17 9:23 ` martin rudalics via Bug reports for GNU Emacs, the Swiss army knife of text editors 0 siblings, 0 replies; 45+ messages in thread From: martin rudalics via Bug reports for GNU Emacs, the Swiss army knife of text editors @ 2024-07-17 9:23 UTC (permalink / raw) To: bjorn.bidar, 59862; +Cc: juri [-- Attachment #1: Type: text/plain, Size: 1456 bytes --] > I meant window. Same as you explain further below in your message > I call quit-window on a dedicate window the frame also dies with it. Note that if a window is dedicated, 'quit-restore-window' will already do its best to delete the window together with its frame. > I think if the window inside of a frame didn't contain any other buffer > than the initial buffer and a new buffer that uses the same window > there could be the argument that the window should die and thus the > frame too. I attach a patch (including all previous changes) that tries to do that. In particular, evaluating (custom-set-variables '(display-buffer-alist '(("\\*info\\*" display-buffer-pop-up-window))) '(quit-restore-window-no-switch t) '(frame-auto-hide-function 'delete-frame)) and doing C-x 5 2 then C-h i then C-x 0 and finally q, will delete the second frame. Alternatively, evaluating (custom-set-variables '(display-buffer-alist '(("\\*info\\*" display-buffer-same-window))) '(quit-restore-window-no-switch 'skip-initial) '(frame-auto-hide-function 'delete-frame)) and doing C-x 5 2 then C-h i and q, will delete the second frame too. Finally, evaluating (custom-set-variables '(display-buffer-alist '(("\\*info\\*" display-buffer-same-window))) '(quit-restore-window-no-switch t) '(frame-auto-hide-function 'delete-frame)) and doing C-x 5 2 then C-h i then C-x k *scratch* and finally q will also delete the second frame. martin [-- Attachment #2: windows-and-buffers.diff --] [-- Type: text/x-patch, Size: 28470 bytes --] diff --git a/lisp/window.el b/lisp/window.el index 9fc2e5d65e8..ad552f5139e 100644 --- a/lisp/window.el +++ b/lisp/window.el @@ -4542,14 +4542,18 @@ record-window-buffer (push-window-buffer-onto-prev window) (run-hooks 'buffer-list-update-hook)))) +;; A version in C that handles dead windows (that presumably are part of +;; a saved window configuration) lives in window.c and is called +;; window_discard_buffer_from_dead_window. We could call that from here +;; but it seems more practical to handle live windows entirely in Elisp. (defun unrecord-window-buffer (&optional 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 must be a live window and defaults to the selected one. BUFFER +must be a live buffer and defaults to the buffer of WINDOW (although +that default hardly makes any sense). -Make BUFFER disappear from all variables mentioned by the object of -WINDOW. This includes the buffers previously shwon in WINDOW as well as +Make BUFFER disappear from most components specified by the object of +WINDOW. This includes the buffers previously shown 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)) @@ -4562,7 +4566,13 @@ unrecord-window-buffer window (assq-delete-all buffer (window-prev-buffers window))) (set-window-next-buffers window (delq buffer (window-next-buffers window))) - + ;; If the fourth elements of the 'quit-restore' or + ;; 'quit-restore-prev' parameters equal BUFFER, these parameters + ;; become useless - in 'quit-restore-window' the fourth element + ;; must equal the buffer of WINDOW in order to use that parameter. + ;; If BUFFER is mentioned in the second element of the parameter, + ;; 'quit-restore-window' cannot possibly show BUFFER instead; so + ;; this parameter becomes useless too. (when (or (eq buffer (nth 3 quit-restore-prev)) (and (listp (setq quad (nth 1 quit-restore-prev))) (eq (car quad) buffer))) @@ -4572,7 +4582,8 @@ unrecord-window-buffer (and (listp (setq quad (nth 1 quit-restore))) (eq (car quad) buffer))) (set-window-parameter - window 'quit-restore (window-parameter window 'quit-restore-prev)))))) + window 'quit-restore (window-parameter window 'quit-restore-prev)) + (set-window-parameter window 'quit-restore-prev nil))))) (defun set-window-buffer-start-and-point (window buffer &optional start point) "Set WINDOW's buffer to BUFFER. @@ -5091,6 +5102,18 @@ previous-buffer (not (or executing-kbd-macro noninteractive))) (user-error "No previous buffer")))))) +(defcustom kill-buffer-quit-windows nil + "Non-nil means killing buffers shall quit windows. +If this is nil, killing a buffer may delete dedicated windows only. If +this is non-nil, `kill-buffer' (and `replace-buffer-in-windows' in +consequence) have `quit-restore-window' deal with any window showing the +buffer to be killed which may delete the window if it's not dedicated to +its buffer. Also, `delete-windows-on' will use `quit-restore-window' as +fallback when a window cannot be deleted otherwise." + :type 'boolean + :version "31.1" + :group 'windows) + (defun delete-windows-on (&optional buffer-or-name frame) "Delete all windows showing BUFFER-OR-NAME. BUFFER-OR-NAME may be a buffer or the name of an existing buffer @@ -5128,8 +5151,10 @@ delete-windows-on 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. +any such case, call either `quit-restore-window' (provided +`kill-buffer-quit-windows' is non-nil) or `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 @@ -5166,9 +5191,18 @@ delete-windows-on ((eq deletable t) ;; Delete window. (delete-window window)) + (kill-buffer-quit-windows + (quit-restore-window window 'bury) + (when (window-live-p window) + ;; Unrecord BUFFER in this window. + (unrecord-window-buffer window buffer))) (t - (when (eq (window-buffer window) buffer) - (quit-restore-window window 'bury)) + ;; 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 (window-live-p window) ;; Unrecord BUFFER in this window. (unrecord-window-buffer window buffer))))) @@ -5177,29 +5211,46 @@ delete-windows-on (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. 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 -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 +BUFFER-OR-NAME may be a buffer or the name of an existing buffer and +defaults to the current buffer. Minibuffer windows are not considered. + +If the option `kill-buffer-quit-windows' is nil, behave as follows: With +the exception of side windows, when a window showing BUFFER-OR-NAME is +dedicated, delete that window. If that window is the only window on its +frame, delete its frame when there are other frames left. In any other +case, call `switch-to-prev-buffer' to display some other buffer in that window. -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." +If `kill-buffer-quit-windows' is non-nil, call `quit-restore-window' for +any window showing BUFFER-OR-NAME with the argument BURY-OR-KILL set to +`killing' to avoid that the latter kills the buffer prematurely. + +In either case, remove the buffer denoted by BUFFER-OR-NAME from the +lists of previous and next buffers of all windows and remove any +`quit-restore' or `quit-restore-prev' parameters mentioning it. + +This function is called by `kill-buffer' which kills the buffer +specified by `buffer-or-name' afterwards. It never kills a buffer by +itself." (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. + ;; Scan all windows. We have to unrecord BUFFER-OR-NAME in those + ;; not showing it. (dolist (window (window-list-1 nil nil t)) (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))))) + (if kill-buffer-quit-windows + (quit-restore-window window 'killing) + (let ((dedicated-side (eq (window-dedicated-p window) 'side))) + (when (or dedicated-side (not (window--delete window t 'kill))) + ;; 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)))))) + + (when (window-live-p window) + ;; 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." @@ -5207,6 +5258,23 @@ quit-window-hook :version "27.1" :group 'windows) +(defcustom quit-restore-window-no-switch nil + "Non-nil means `quit-restore-window' preferably won't switch buffers. +If this is nil, `quit-restore-window' will call `switch-to-prev-buffer' +unless the window has been made by `display-buffer'. If this is t, +`quit-restore-window' will switch to a previous buffer only if a live +buffer exists that was previously shown in that window. If this is the +symbol `skip-initial', it will not switch to the initial buffer of the +window's `quit-restore' parameter. + +In either case, if `quit-restore-window' doesn't switch to a previous +buffer, it will try to delete the window (and maybe its frame) instead. +Note also that if a window is dedicated, `quit-restore-window' will +usually not switch to a previous buffer in it." + :type 'boolean + :version "31.1" + :group 'windows) + (defun window--quit-restore-select-window (window) "Select WINDOW after having quit another one. Do not select an inactive minibuffer window." @@ -5227,9 +5295,10 @@ quit-restore-window 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 -deleted. +If WINDOW's dedicated flag is t, try to delete WINDOW. If it equals the +value `side', restore that value when WINDOW is not deleted. Whether +WINDOW or its frame get deleted can be further controlled via the option +`quit-restore-window-no-switch'. Optional second argument BURY-OR-KILL tells how to proceed with the buffer of WINDOW. The following values are handled: @@ -5249,7 +5318,14 @@ quit-restore-window most reliable remedy to not have `switch-to-prev-buffer' switch to this buffer again without killing the buffer. -`kill' means to kill WINDOW's buffer." +`kill' means to kill WINDOW's buffer. + +`killing' is like `kill' but means that WINDOW's buffer will get killed +elsewhere. This value is used by `replace-buffer-in-windows' and +`quit-windows-on'. + +`burying' is like `bury' but means that WINDOW's buffer will get buried +elsewhere. This value is used by `quit-windows-on'." (setq window (window-normalize-window window t)) (let* ((buffer (window-buffer window)) (quit-restore (window-parameter window 'quit-restore)) @@ -5265,32 +5341,38 @@ quit-restore-window (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))) + (window--delete + window 'dedicated (memq bury-or-kill '(kill killing)))) ;; If the previously selected window is still alive, select it. (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)) - ((and (not prev-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))) + ((and (or (not prev-buffer) + ;; Ignore first of the previous buffers if + ;; 'quit-restore-window-no-switch' says so. + (and (eq quit-restore-window-no-switch 'skip-initial) + (not (cdr (window-prev-buffers window))))) + (or (eq (nth 1 quit-restore) 'frame) + ;; 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). + (and (eq (nth 1 quit-restore) 'window) + (not (eq window (frame-root-window window)))) + ;; But always allow deleting a frame or window if + ;; 'quit-restore-window-no-switch' says so. + quit-restore-window-no-switch) + (or (eq (nth 3 quit-restore) buffer) + quit-restore-window-no-switch) ;; Delete WINDOW if possible. - (window--delete window nil (eq bury-or-kill 'kill))) + (window--delete + window nil (memq bury-or-kill '(kill killing)))) ;; If the previously selected window is still alive, select it. (window--quit-restore-select-window quit-restore-2)) ((or (and (listp (setq quad (nth 1 quit-restore-prev))) @@ -5328,7 +5410,7 @@ quit-restore-window ;; 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 + (when (memq bury-or-kill '(bury burying kill killing)) ;; Remove buffer from WINDOW's previous and next buffers. (set-window-prev-buffers window (assq-delete-all buffer (window-prev-buffers window))) @@ -5346,7 +5428,6 @@ quit-restore-window ;; 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 @@ -5359,13 +5440,14 @@ 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))))) + (window--delete + window nil (memq bury-or-kill '(kill killing)))))) ;; Deal with the buffer. (cond ((not (buffer-live-p buffer))) ((eq bury-or-kill 'kill) (kill-buffer buffer)) - (bury-or-kill + ((eq bury-or-kill 'bury) (bury-buffer-internal buffer))))) (defun quit-window (&optional kill window) @@ -5406,11 +5488,18 @@ quit-windows-on (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)) + (quit-restore-window + window (if kill 'killing 'burying))) (when (window-live-p window) ;; Unrecord BUFFER in this window. - (unrecord-window-buffer window buffer))))) + (unrecord-window-buffer window buffer))) + + ;; Deal with BUFFER-OR-NAME. + (cond + ((not (buffer-live-p buffer))) + (kill (kill-buffer buffer)) + (t (bury-buffer-internal buffer))))) \f (defun window--combination-resizable (parent &optional horizontal) "Return number of pixels recoverable from height of window PARENT. @@ -6723,36 +6812,38 @@ display-buffer-record-window 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." +update a `quit-restore-prev' parameter for this window. This allows for +quitting WINDOW in a similar fashion but also keeps the very first +`quit-restore' parameter stored for this window around. Consequently, +WINDOW (or its frame) can be eventually deleted by `quit-restore-widow' +if that parameter's fourth element equals WINDOW's buffer." (cond ((eq type 'reuse) - (if (eq (window-buffer window) buffer) - ;; WINDOW shows BUFFER already. Update WINDOW's quit-restore - ;; parameter, if any. - (let ((quit-restore (window-parameter window 'quit-restore))) + (let ((quit-restore (window-parameter window 'quit-restore))) + (if (eq (window-buffer window) buffer) + ;; WINDOW shows BUFFER already. Update WINDOW's quit-restore + ;; parameter, if any. (when (consp quit-restore) (setcar quit-restore 'same) ;; The selected-window might have changed in ;; between (Bug#20353). (unless (or (eq window (selected-window)) - (eq window (nth 2 quit-restore))) - (setcar (cddr quit-restore) (selected-window))))) - ;; WINDOW shows another buffer. - (with-current-buffer (window-buffer window) - (set-window-parameter - window 'quit-restore-prev - (list 'other - ;; A quadruple of WINDOW's buffer, start, point and height. - (list (current-buffer) (window-start window) - ;; Preserve window-point-insertion-type (Bug#12855). - (copy-marker - (window-point window) window-point-insertion-type) - (if (window-combined-p window) - (window-total-height window) - (window-total-width window))) - (selected-window) buffer))))) + (eq window (nth 2 quit-restore))) + (setcar (cddr quit-restore) (selected-window)))) + ;; WINDOW shows another buffer. + (with-current-buffer (window-buffer window) + (set-window-parameter + window (if quit-restore 'quit-restore-prev 'quit-restore) + (list 'other + ;; A quadruple of WINDOW's buffer, start, point and height. + (list (current-buffer) (window-start window) + ;; Preserve window-point-insertion-type (Bug#12855). + (copy-marker + (window-point window) window-point-insertion-type) + (if (window-combined-p window) + (window-total-height window) + (window-total-width window))) + (selected-window) buffer)))))) ((eq type 'window) ;; WINDOW has been created on an existing frame. (set-window-parameter diff --git a/src/alloc.c b/src/alloc.c index 666f77bfce1..b955651f5c0 100644 --- a/src/alloc.c +++ b/src/alloc.c @@ -6998,33 +6998,6 @@ mark_face_cache (struct face_cache *c) } } -/* Remove killed buffers or items whose car is a killed buffer from - LIST, and mark other items. Return changed LIST, which is marked. */ - -static Lisp_Object -mark_discard_killed_buffers (Lisp_Object list) -{ - Lisp_Object tail, *prev = &list; - - for (tail = list; CONSP (tail) && !cons_marked_p (XCONS (tail)); - tail = XCDR (tail)) - { - Lisp_Object tem = XCAR (tail); - if (CONSP (tem)) - tem = XCAR (tem); - if (BUFFERP (tem) && !BUFFER_LIVE_P (XBUFFER (tem))) - *prev = XCDR (tail); - else - { - set_cons_marked (XCONS (tail)); - mark_object (XCAR (tail)); - prev = xcdr_addr (tail); - } - } - mark_object (tail); - return list; -} - static void mark_frame (struct Lisp_Vector *ptr) { @@ -7079,15 +7052,6 @@ mark_window (struct Lisp_Vector *ptr) mark_glyph_matrix (w->current_matrix); mark_glyph_matrix (w->desired_matrix); } - - /* Filter out killed buffers from both buffer lists - in attempt to help GC to reclaim killed buffers faster. - We can do it elsewhere for live windows, but this is the - best place to do it for dead windows. */ - wset_prev_buffers - (w, mark_discard_killed_buffers (w->prev_buffers)); - wset_next_buffers - (w, mark_discard_killed_buffers (w->next_buffers)); } /* Entry of the mark stack. */ diff --git a/src/buffer.c b/src/buffer.c index 744b0ef5548..6ec40aff646 100644 --- a/src/buffer.c +++ b/src/buffer.c @@ -2012,6 +2012,13 @@ DEFUN ("kill-buffer", Fkill_buffer, Skill_buffer, 0, 1, "bKill buffer: ", buffer (bug#10114). */ replace_buffer_in_windows (buffer); + /* For dead windows that have not been collected yet, remove this + buffer from those windows' lists of previously and next shown + buffers and remove any 'quit-restore' or 'quit-restore-prev' + parameters mentioning the buffer. */ + if (XFIXNUM (BVAR (b, display_count)) > 0) + window_discard_buffer_from_dead_windows (buffer); + /* Exit if replacing the buffer in windows has killed our buffer. */ if (!BUFFER_LIVE_P (b)) return Qt; diff --git a/src/window.c b/src/window.c index ff28bac5306..dd0abe84fdf 100644 --- a/src/window.c +++ b/src/window.c @@ -3277,6 +3277,90 @@ window_pixel_to_total (Lisp_Object frame, Lisp_Object horizontal) } +/* Remove first occurrence of element whose car is BUFFER from ALIST. + Return changed ALIST. */ +static Lisp_Object +window_discard_buffer_from_alist (Lisp_Object buffer, Lisp_Object alist) +{ + Lisp_Object tail, *prev = &alist; + + for (tail = alist; CONSP (tail); tail = XCDR (tail)) + { + Lisp_Object tem = XCAR (tail); + + tem = XCAR (tem); + + if (EQ (tem, buffer)) + { + *prev = XCDR (tail); + break; + } + else + prev = xcdr_addr (tail); + } + + return alist; +} + +/* Remove first occurrence of BUFFER from LIST. Return changed + LIST. */ +static Lisp_Object +window_discard_buffer_from_list (Lisp_Object buffer, Lisp_Object list) +{ + Lisp_Object tail, *prev = &list; + + for (tail = list; CONSP (tail); tail = XCDR (tail)) + { + if (EQ (XCAR (tail), buffer)) + { + *prev = XCDR (tail); + break; + } + else + prev = xcdr_addr (tail); + } + + return list; +} + +static void +window_discard_buffer_from_dead_window (Lisp_Object buffer, Lisp_Object window) +{ + struct window *w = XWINDOW (window); + Lisp_Object quit_restore = window_parameter (w, Qquit_restore); + Lisp_Object quit_restore_prev = window_parameter (w, Qquit_restore_prev); + Lisp_Object quad; + + wset_prev_buffers + (w, window_discard_buffer_from_alist (buffer, w->prev_buffers)); + wset_next_buffers + (w, window_discard_buffer_from_list (buffer, w->next_buffers)); + + if (EQ (buffer, Fnth (make_fixnum (3), quit_restore_prev)) + || (CONSP (quad = Fcar (Fcdr (quit_restore_prev))) + && EQ (Fcar (quad), buffer))) + Fset_window_parameter (window, Qquit_restore_prev, Qnil); + + if (EQ (buffer, Fnth (make_fixnum (3), quit_restore)) + || (CONSP (quad = Fcar (Fcdr (quit_restore))) + && EQ (Fcar (quad), buffer))) + { + Fset_window_parameter (window, Qquit_restore, + window_parameter (w, Qquit_restore_prev)); + Fset_window_parameter (window, Qquit_restore_prev, Qnil); + } +} + +void +window_discard_buffer_from_dead_windows (Lisp_Object buffer) +{ + struct Lisp_Hash_Table *h = XHASH_TABLE (window_dead_windows_table); + + DOHASH (h, k, v) + window_discard_buffer_from_dead_window (buffer, v); +} + + DEFUN ("delete-other-windows-internal", Fdelete_other_windows_internal, Sdelete_other_windows_internal, 0, 2, "", doc: /* Make WINDOW fill its frame. @@ -4402,6 +4486,10 @@ make_parent_window (Lisp_Object window, bool horflag) wset_buffer (p, Qnil); wset_combination (p, horflag, window); wset_combination_limit (p, Qnil); + /* Reset any previous and next buffers of p which have been installed + by the memcpy above. */ + wset_prev_buffers (p, Qnil); + wset_next_buffers (p, Qnil); wset_window_parameters (p, Qnil); } @@ -4426,10 +4514,6 @@ make_window (void) wset_vertical_scroll_bar_type (w, Qt); wset_horizontal_scroll_bar_type (w, Qt); wset_cursor_type (w, Qt); - /* These Lisp fields are marked specially so they're not set to nil by - allocate_window. */ - wset_prev_buffers (w, Qnil); - wset_next_buffers (w, Qnil); /* Initialize non-Lisp data. Note that allocate_window zeroes out all non-Lisp data, so do it only for slots which should not be zero. */ @@ -5252,6 +5336,11 @@ DEFUN ("delete-window-internal", Fdelete_window_internal, Sdelete_window_interna unchain_marker (XMARKER (w->old_pointm)); unchain_marker (XMARKER (w->start)); wset_buffer (w, Qnil); + /* Add WINDOW to table of dead windows so when killing a buffer + WINDOW mentions, all references to that buffer can be removed + and the buffer be collected. */ + Fputhash (make_fixnum (w->sequence_number), + window, window_dead_windows_table); } if (NILP (s->prev) && NILP (s->next)) @@ -7356,6 +7445,10 @@ DEFUN ("set-window-configuration", Fset_window_configuration, } } + /* Remove window from the table of dead windows. */ + Fremhash (make_fixnum (w->sequence_number), + window_dead_windows_table); + if ((NILP (dont_set_miniwindow) || !MINI_WINDOW_P (w)) && BUFFERP (p->buffer) && BUFFER_LIVE_P (XBUFFER (p->buffer))) /* If saved buffer is alive, install it, unless it's a @@ -7585,6 +7678,11 @@ delete_all_child_windows (Lisp_Object window) possible resurrection in Fset_window_configuration. */ wset_combination_limit (w, w->contents); wset_buffer (w, Qnil); + /* Add WINDOW to table of dead windows so when killing a buffer + WINDOW mentions, all references to that buffer can be removed + and the buffer be collected. */ + Fputhash (make_fixnum (w->sequence_number), + window, window_dead_windows_table); } Vwindow_list = Qnil; @@ -8594,6 +8692,8 @@ syms_of_window (void) DEFSYM (Qconfiguration, "configuration"); DEFSYM (Qdelete, "delete"); DEFSYM (Qdedicated, "dedicated"); + DEFSYM (Qquit_restore, "quit-restore"); + DEFSYM (Qquit_restore_prev, "quit-restore-prev"); DEFVAR_LISP ("temp-buffer-show-function", Vtemp_buffer_show_function, doc: /* Non-nil means call as function to display a help buffer. @@ -8917,6 +9017,17 @@ syms_of_window (void) displayed after a scrolling operation to be somewhat inaccurate. */); fast_but_imprecise_scrolling = false; + DEFVAR_LISP ("window-dead-windows-table", window_dead_windows_table, + doc: /* Hash table of dead windows. +Each entry in this table maps a window number to a window object. +Entries are added by `delete-window-internal' and are removed by the +garbage collector. + +This table is maintained by code in window.c and is made visible in +Elisp for testing purposes only. */); + window_dead_windows_table + = CALLN (Fmake_hash_table, QCweakness, Qt); + defsubr (&Sselected_window); defsubr (&Sold_selected_window); defsubr (&Sminibuffer_window); diff --git a/src/window.h b/src/window.h index 86932181252..335e0a3453e 100644 --- a/src/window.h +++ b/src/window.h @@ -142,6 +142,12 @@ #define WINDOW_H_INCLUDED as well. */ Lisp_Object contents; + /* A list of <buffer, window-start, window-point> triples listing + buffers previously shown in this window. */ + Lisp_Object prev_buffers; + /* List of buffers re-shown in this window. */ + Lisp_Object next_buffers; + /* The old buffer of this window, set to this window's buffer by run_window_change_functions every time it sees this window. Unused for internal windows. */ @@ -218,14 +224,6 @@ #define WINDOW_H_INCLUDED struct glyph_matrix *current_matrix; struct glyph_matrix *desired_matrix; - /* The two Lisp_Object fields below are marked in a special way, - which is why they're placed after `current_matrix'. */ - /* A list of <buffer, window-start, window-point> triples listing - buffers previously shown in this window. */ - Lisp_Object prev_buffers; - /* List of buffers re-shown in this window. */ - Lisp_Object next_buffers; - /* Number saying how recently window was selected. */ EMACS_INT use_time; @@ -1228,6 +1226,7 @@ #define CHECK_LIVE_WINDOW(WINDOW) \ extern void wset_buffer (struct window *, Lisp_Object); extern bool window_outdated (struct window *); extern ptrdiff_t window_point (struct window *w); +extern void window_discard_buffer_from_dead_windows (Lisp_Object); extern void init_window_once (void); extern void init_window (void); extern void syms_of_window (void); ^ permalink raw reply related [flat|nested] 45+ messages in thread
* bug#59862: quit-restore per window buffer 2024-07-14 7:49 ` Juri Linkov 2024-07-15 7:32 ` martin rudalics via Bug reports for GNU Emacs, the Swiss army knife of text editors @ 2024-07-30 8:20 ` martin rudalics via Bug reports for GNU Emacs, the Swiss army knife of text editors 2024-07-31 17:30 ` Juri Linkov 1 sibling, 1 reply; 45+ messages in thread From: martin rudalics via Bug reports for GNU Emacs, the Swiss army knife of text editors @ 2024-07-30 8:20 UTC (permalink / raw) To: Juri Linkov; +Cc: 59862 [-- Attachment #1: Type: text/plain, Size: 5649 bytes --] > OTOH, I tried to add the tab case handling to the same places > where the frame case is handled, and everything works nicely > with this patch applied over your previous patches: After fixing a few other inconsistencies find my final version of the patch here. The ChangeLog is below. If you see no further problems, I'll install it and you can add your changes on top of it. martin Improve window/buffer handling code The purpose of these changes is to improve the code handling the display of buffers in windows, switching to previous and next buffers in windows and restoring a previous state after quitting or killing buffers. In particular it does: - Add a new window parameter 'quit-restore-prev' so a window can keep its initial 'quit-restore' parameter and undoing a sequence of quit window operations becomes more reliable (Bug#59862). - Optionally have 'kill-buffer' call 'quit-restore-window' for all windows showing the argument buffer (Bug#59862). - Add a new hook so it's possible to avoid that a window gets deleted implicitly by functions like 'kill-buffer' (Bug#71386). - Add a new option to make 'quit-restore-window' delete windows more aggressively (Bug#59862). - Immediately remove killed buffers from all windows' previous and next buffers. For windows that are already dead, use a weak hash table to be used by 'kill-buffer'. This avoids any special handling of such windows by the garbage collector. - Immediately remove 'quit-restore' and 'quit-restore-prev' window parameters that reference killed buffers. These parameters have no more use once their buffers got killed. - Make sure that internal windows do not have any previous and next buffers. This fixes a silly memory leak. - Make sure that after set_window_buffer and some wset_buffer calls the buffer now shown in the window does not appear in the lists of that window's previous and next buffers. The old behavior could make functions investigating these lists erroneously believe that there still existed some other buffer to switch to. * src/alloc.c (mark_discard_killed_buffers): Remove function. (mark_window): No more filter previous and next buffer lists. * src/window.h (struct window): Move up prev_buffers and next-buffers in structure; they are now treated by the collector as usual. * src/window.c (window_discard_buffer_from_alist) (window_discard_buffer_from_list) (window_discard_buffer_from_window) (window_discard_buffer_from_dead_windows) (Fwindow_discard_buffer): New functions. (set_window_buffer): Discard BUFFER from WINDOW's previous and next buffers. (make_parent_window): Make sure internal windows have no previous and next buffers. (make_window): Don't initialize window's previous and next buffers, they are handled by allocate_window now. (Fdelete_window_internal): Add WINDOW to window_dead_windows_table. (Fset_window_configuration): Remove resurrected window from window_dead_windows_table. Make sure buffers set by wset_buffer calls are not recorded in window's previous and next buffers. (delete_all_child_windows): Add deleted windows to window_dead_windows_table. (window_dead_windows_table): New weak hash table to record dead windows that are stored in saved window configurations. * src/buffer.c (Fkill_buffer): Call new function 'window_discard_buffer_from_dead_windows'. * lisp/window.el (window-deletable-functions): New hook. (window-deletable-p): Update doc-string. Run 'window-deletable-functions' (Bug#71386). (unrecord-window-buffer): New argument ALL. Move body to 'window-discard-buffer-from-window' so that if ALL is non-nil, WINDOW's 'quit-restore' and 'quit-restore-prev' parameters get removed too. (switch-to-prev-buffer): Don't care about killed buffers here; 'replace-buffer-in-windows' should have done that already. Use 'unrecord-window-buffer'. (switch-to-next-buffer): Don't care about killed buffers here; 'replace-buffer-in-windows' should do that now. (kill-buffer-quit-windows): New option. (delete-windows-on): Update doc-string. Handle new option 'kill-buffer-quit-windows'. Update 'unrecord-window-buffer' calls. (replace-buffer-in-windows): Update doc-string. Handle new option 'kill-buffer-quit-windows' (Bug#59862). Update call to 'unrecord-window-buffer'. (quit-restore-window-no-switch): New option. (quit-restore-window): Update doc-string. Handle additional values of BURY-OR-KILL so to not kill a buffer about to be killed by the caller. Handle 'quit-restore-prev' parameter (Bug#59862). Handle new option 'quit-restore-window-no-switch' (Bug#59862). (quit-windows-on): Update doc-string. Call 'quit-window-hook' and call 'quit-restore-window' directly so that the buffer does not get buried or killed by the latter. Update 'unrecord-window-buffer' call. (display-buffer-record-window): Update doc-string. Handle new `quit-restore-prev' parameter (Bug#59862). (switch-to-buffer): Call 'display-buffer-record-window' so a latter 'quit-restore-window' can use its parameters. * doc/lispref/windows.texi (Deleting Windows): Describe implicit deletion of windows and new hook 'window-deletable-functions'. (Buffers and Windows): Update description of 'replace-buffer-in-windows'. Describe new option 'kill-buffer-quit-windows'. (Quitting Windows): Describe 'quit-restore-prev' parameter and new option 'quit-restore-window-no-switch'. Update description of 'quit-restore-window'. (Window Parameters): Mention 'quit-restore-prev' parameter. * etc/NEWS: Add entries for 'window-deletable-functions', 'kill-buffer-quit-windows', 'quit-restore-window-no-switch'. mention new parameter 'quit-restore-prev' and new argument values for 'quit-restore-window'. [-- Attachment #2: window-buffer-changes.diff --] [-- Type: text/x-patch, Size: 58275 bytes --] diff --git a/doc/lispref/windows.texi b/doc/lispref/windows.texi index f5963d984e9..656a44dfcbf 100644 --- a/doc/lispref/windows.texi +++ b/doc/lispref/windows.texi @@ -1647,6 +1647,45 @@ Deleting Windows are the opposite of what they are in those other functions. @end deffn +@cindex implicit deletion of windows +@cindex deleting windows implicitly + + The above commands delete windows explicitly. However, Emacs may also +delete a window implicitly when it thinks that it's more intuitive to +eliminate it rather than showing some unrelated buffer in it. Functions +that may delete windows implicitly are @code{kill-buffer} +(@pxref{Killing Buffers}), @code{quit-restore-window} (@pxref{Quitting +Windows}) and @code{bury-buffer} (@pxref{Buffer List}). Some of these +delete a window if and only if that window is dedicated to its buffer +(@pxref{Dedicated Windows}). Others delete a window when that window +has been created by @code{display-buffer} (@pxref{Displaying Buffers}). +Some will also try to delete a window's frame together with the window, +provided there are other frames on the same terminal and the frame does +not host the active minibuffer window. + + The hook described next can be used to avoid that a window gets +deleted by these functions. + +@defopt window-deletable-functions +This is an abnormal hook that can be used to avoid that a window gets +deleted implicitly. The value should be a list of functions that take +two arguments. The first argument is the window about to be deleted. +The second argument, if non-@code{nil}, means that the window is the +only window on its frame and would be deleted together with its frame. +The window's buffer is current when running this hook. + +If any of these functions returns @code{nil}, the window will not be +deleted and another buffer will be shown in it. This hook is run +(indirectly) by the functions @code{quit-restore-window}, +@code{kill-buffer} and @code{bury-buffer}. It is not run by functions +that delete windows explicitly like @code{delete-window}, +@code{delete-other-windows} or @code{delete-windows-on}. + +The purpose of this hook is to give its clients a chance to save a +window or its frame from deletion because they might still want to use +that window or frame for their own purposes. +@end defopt + @node Recombining Windows @section Recombining Windows @@ -2297,16 +2336,41 @@ Buffers and Windows the name of an existing buffer; if omitted or @code{nil}, it defaults to the current buffer. -The replacement buffer in each window is chosen via +The replacement buffer in each window is usually chosen via @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 +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. + +The main purpose of this function is to decide what to do with windows +whose buffers are about to be killed by @code{kill-buffer} +(@pxref{Killing Buffers}). It will, however, also remove the buffer +specified by @var{buffer-or-name} from the lists of previous and next +buffers (@pxref{Window History}) of all windows (including dead windows +that are only referenced by window configurations) and remove any +@code{quit-restore} or @code{quit-restore-prev} parameters +(@pxref{Window Parameters}) referencing that buffer. @end deffn +By default, @code{replace-buffer-in-windows} deletes only windows +dedicated to their buffers and ignores any @code{quit-restore} or +@code{quit-restore-prev} parameters of the windows it works on. The +following option is useful for circumventing these restrictions. + +@defopt kill-buffer-quit-windows +If this option is @code{nil}, @code{kill-buffer} (and in consequence +@code{replace-buffer-in-windows}) may only delete windows that are +dedicated to the buffer about to be killed. If this is non-@code{nil}, +@code{replace-buffer-in-windows} has @code{quit-restore-window} +(@pxref{Quitting Windows}) deal with any such window. That function may +delete such a window even if it's not dedicated to its buffer. Also, +@code{delete-windows-on} will use @code{quit-restore-window} as fallback +when a window cannot be deleted and another buffer must be shown in it. +@end defopt + @node Switching Buffers @section Switching to a Buffer in a Window @@ -4532,6 +4596,15 @@ Quitting Windows @item kill This means to kill @var{window}'s buffer. + +@item killing +This is handled like @code{kill} but assumes that @var{window}'s buffer +gets killed elsewhere. This value is used by +@code{replace-buffer-in-windows} and @code{quit-windows-on}. + +@item burying +This is handled like @code{bury} but assumes that @var{window}'s buffer +gets buried elsewhere. This value is used by @code{quit-windows-on}. @end table The argument @var{bury-or-kill} also specifies what to do with @@ -4541,14 +4614,18 @@ Quitting Windows delete the frame. Otherwise, the fate of the frame is determined by calling @code{frame-auto-hide-function} (see below) with that frame as sole argument. - -This function always sets @var{window}'s @code{quit-restore} parameter -to @code{nil} unless it deletes the window. @end defun -The window @var{window}'s @code{quit-restore} parameter (@pxref{Window -Parameters}) should be @code{nil} or a list of four elements: -@c FIXME: describe what quit-restore-window does if this is nil. + A window's @code{quit-restore} and @code{quit-restore-prev} parameters +(@pxref{Window Parameters}) guide @code{quit-restore-window} through its +process of dealing with its @var{window} argument. If such a parameter +is absent or @code{nil}, this usually means that the window has been +created by a command like @code{split-window-below} or +@code{split-window-right} (@pxref{Split Window,,, emacs, The GNU Emacs +Manual}) and @code{quit-restore-window} will delete the window only if +it was dedicated to its buffer. + + If non-@code{nil}, any such parameter is a list of four elements: @lisp (@var{method} @var{obuffer} @var{owindow} @var{this-buffer}) @@ -4586,17 +4663,17 @@ Quitting Windows @var{window}, it tries to select @var{owindow}. The fourth element, @var{this-buffer}, is the buffer whose displaying -set the @code{quit-restore} parameter. Quitting @var{window} may delete -that window only if it still shows that buffer. - -Quitting @var{window} tries to delete it if and only if (1) -@var{method} is either @code{window} or @code{frame}, (2) the window -has no history of previously-displayed buffers and (3) -@var{this-buffer} equals the buffer currently displayed in -@var{window}. If @var{window} is part of an atomic window -(@pxref{Atomic Windows}), quitting will try to delete the root of that -atomic window instead. In either case, it tries to avoid signaling an -error when @var{window} cannot be deleted. +set the @code{quit-restore} parameter. Quitting @var{window} will use +the information stored by that parameter if and only if it still shows +that buffer. + +@code{quit-restore-window} tries to delete its @var{window} if (1) +@var{method} is either @code{window} or @code{frame}, (2) the window has +no history of previously-displayed buffers and (3) @var{this-buffer} +equals the buffer currently displayed in @var{window}. If @var{window} +is part of an atomic window (@pxref{Atomic Windows}), it will try to +delete the root of that atomic window instead. In either case, it tries +to avoid signaling an error when @var{window} cannot be deleted. If @var{obuffer} is a list, and @var{prev-buffer} is still live, quitting displays @var{prev-buffer} in @var{window} according to the @@ -4608,6 +4685,58 @@ Quitting Windows buffers (@pxref{Window History}), the most recent buffer in that history will be displayed. + Conceptually, the @code{quit-restore} parameter is used for undoing +the first buffer display operation for a specific window. The +@code{quit-restore-prev} parameter is used for undoing the last buffer +display operation in a row of such operations for a specific window. +Any buffer display operation for that window that happened in between, +is undone by displaying the buffer previously shown in that window. + + @code{display-buffer} sets up the @code{quit-restore} parameter of any +window it uses when that window has been either created by it or has no +non-@code{nil} @code{quit-restore} parameter. If the window already has +a @code{quit-restore} parameter, @code{display-buffer} adds a +@code{quit-restore-prev} parameter whose @var{method} element is either +@code{same} or @code{other}. In either case, if the window already has +a @code{quit-restore-prev} or @code{quit-restore} parameter, it may +update that parameter's contents. + + @code{quit-restore-window} now first tries to find a suitable +@code{quit-restore-prev} parameter for its window telling which buffer +to show instead. If it doesn't find one, it will look for a suitable +@code{quit-restore} parameter to either delete the window or show +another buffer in it. + + Once it has used one of these parameters, @code{quit-restore-window} +resets it to @code{nil}. Parameters it did not use are left alone. Any +of these parameters are also reset by @code{replace-buffer-in-windows} +(@pxref{Buffers and Windows}) when they reference a buffer that is about +to be killed, either as the buffer specified by @var{prev-buffer} or as +the buffer specified by @var{this-buffer}. + + All operations on these parameters are supposed to preserve the +following invariant: If a window has a non-@code{nil} +@code{quit-restore-prev} parameter, it also has a non-@code{nil} +@code{quit-restore} parameter. + +The following option allows @code{quit-restore-window} to delete its +window more aggressively. + +@defopt quit-restore-window-no-switch +If this option is @code{nil}, @code{quit-restore-window} will always +call @code{switch-to-prev-buffer} unless the window has been made by +@code{display-buffer}. If this is @code{t}, @code{quit-restore-window} +will try hard to switch only to buffers previously shown in that window. +If this is the symbol @code{skip-first}, it will switch to a previous +buffer if and only the window has at least two previous buffers. + +In either case, if @code{quit-restore-window} doesn't switch to a +previous buffer, it will try to delete the window (and maybe its +frame) instead. Note also that if a window is dedicated, +@code{quit-restore-window} will usually not switch to a previous +buffer in it either. +@end defopt + @ignore @c FIXME: Should we document display-buffer-reuse-window? If we document display-buffer-record-window, it should be with @defun. @@ -6668,12 +6797,14 @@ Window Parameters @code{window-preserve-size} (@pxref{Preserving Window Sizes}). @item quit-restore +@item quit-restore-prev @vindex quit-restore@r{, a window parameter} -This parameter is installed by the buffer display functions +@vindex quit-restore-prev@r{, a window parameter} +These parameters ares installed by the buffer display functions (@pxref{Choosing Window}) and consulted by @code{quit-restore-window} -(@pxref{Quitting Windows}). It is a list of four elements, see the -description of @code{quit-restore-window} in @ref{Quitting Windows} -for details. +(@pxref{Quitting Windows}). They are lists of four elements, see the +description of @code{quit-restore-window} in @ref{Quitting Windows} for +details. @item window-side @itemx window-slot diff --git a/etc/NEWS b/etc/NEWS index c907ec40fa1..de7ae7b0326 100644 --- a/etc/NEWS +++ b/etc/NEWS @@ -43,6 +43,37 @@ The 'find-function', 'find-library', 'find-face-definition', and 'find-variable' commands now allow retrieving previous input using the usual minibuffer history commands. Each command has a separate history. +\f +** Windows + ++++ +*** New hook 'window-deletable-functions'. +This abnormal hook gives its client a way to save a window from getting +deleted implicitly by functions like 'kill-buffer', 'bury-buffer' and +'quit-restore-window', + ++++ +*** New option 'kill-buffer-quit-windows'. +This option has 'kill-buffer' call 'quit-restore-window' to handle the +further destiny of any window showing the buffer to be killed. + ++++ +*** New window parameter 'quit-restore-prev'. +This parameter is set up by 'display-buffer' when it detects that the +window used already has a 'quit-restore' parameter. Its presence gives +'quit-restore-window' a way to undo a sequence of buffer display +operations more intuitively. + ++++ +*** 'quit-restore-window' now handles the values 'killing' and 'burying' +for its BURY-OR-KILL argument just like 'kill' and 'bury' but assumes +that the actual killing or burying of the buffer is done by the caller. + ++++ +*** New option 'quit-restore-window-no-switch'. +With this option set, 'quit-restore-window' will delete its window more +aggressively rather than switching to some other buffer in it. + \f * Editing Changes in Emacs 31.1 diff --git a/lisp/window.el b/lisp/window.el index 006cfa19525..4687860db11 100644 --- a/lisp/window.el +++ b/lisp/window.el @@ -4104,12 +4104,35 @@ one-window-p (next-window base-window (if nomini 'arg) all-frames)))) \f ;;; Deleting windows. -(defun window-deletable-p (&optional window) +(defcustom window-deletable-functions nil + "Abnormal hook to decide whether a window may be implicitly deleted. +The value should be a list of functions that take two arguments. The +first argument is the window about to be deleted. The second argument +if non-nil, means that the window is the only window on its frame and +should be deleted together with its frame. The window's buffer is +current when running this hook. + +If any of these functions returns nil, the window will not be deleted +and another buffer will be shown in it. This hook is run implicitly by +the functions `quit-restore-window', `kill-buffer' and `bury-buffer'. +It is not run by `delete-window' and `delete-windows-on'. The purpose +of this hook is to give its clients a chance to save a window or its +frame from deletion because they might still want to use that window or +frame for their own purposes." + :type 'hook + :version "31.1" + :group 'windows) + +(defun window-deletable-p (&optional window no-run) "Return t if WINDOW can be safely deleted from its frame. WINDOW must be a valid window and defaults to the selected one. Return `frame' if WINDOW is the root window of its frame and that -frame can be safely deleted." +frame can be safely deleted. + +Unless the optional argument NO-RUN is non-nil, run the abnormal hook +`window-deletable-functions' and return nil if any function on that hook +returns nil." (setq window (window-normalize-window window)) (unless (or ignore-window-parameters @@ -4137,14 +4160,22 @@ window-deletable-p (and minibuf (eq frame (window-frame minibuf)) (not (eq (default-toplevel-value 'minibuffer-follows-selected-frame) - t))))) + t)))) + (or no-run + (not (with-current-buffer (window-buffer window) + (run-hook-with-args-until-failure + 'window-deletable-functions window t))))) 'frame)) ((window-minibuffer-p window) ;; If WINDOW is the minibuffer window of a non-minibuffer-only ;; frame, it cannot be deleted separately. nil) - ((or ignore-window-parameters - (not (eq window (window-main-window frame)))) + ((and (or ignore-window-parameters + (not (eq window (window-main-window frame)))) + (or no-run + (with-current-buffer (window-buffer window) + (run-hook-with-args-until-failure + 'window-deletable-functions window nil)))) ;; Otherwise, WINDOW can be deleted unless it is the main window ;; of its frame. t)))) @@ -4515,17 +4546,26 @@ record-window-buffer (push-window-buffer-onto-prev window) (run-hooks 'buffer-list-update-hook)))) -(defun unrecord-window-buffer (&optional window buffer) +(defun unrecord-window-buffer (&optional window buffer all) "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 must be a live window and defaults to the selected one. BUFFER +must be a live buffer and defaults to the buffer of WINDOW (although +that default hardly makes any sense). + +Make BUFFER disappear from most components specified by the object of +WINDOW. This includes the buffers previously shown in WINDOW as well as +any buffers mentioned by WINDOW's `quit-restore' and `quit-restore-prev' +parameters. + +This function is called by `replace-buffer-in-windows' which is mainly +concerned with finding another buffer for all windows showing a buffer +about to be killed. It's also called by `delete-windows-on' and +`quit-windows-on' and should be called wherever the traces of a buffer +should be erased from the window handling subsystem." (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) + (window-discard-buffer-from-window buffer window all)))) (defun set-window-buffer-start-and-point (window buffer &optional start point) "Set WINDOW's buffer to BUFFER. @@ -4683,7 +4723,7 @@ switch-to-prev-buffer ((or switch-to-prev-buffer-skip (not switch-to-visible-buffer)) frame))) - entry new-buffer killed-buffers skipped) + entry new-buffer skipped) (when (window-minibuffer-p window) ;; Don't switch in minibuffer window. (unless (setq window (minibuffer-selected-window)) @@ -4699,14 +4739,14 @@ switch-to-prev-buffer (dolist (entry (window-prev-buffers window)) (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)))) - (or (null pred) (funcall pred new-buffer)) + ;; Beware: new-buffer might have been killed by + ;; a function on 'buffer-predicate'. + (buffer-live-p new-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. (or bury-or-kill (not (memq new-buffer next-buffers)))) - (if (switch-to-prev-buffer-skip-p skip window new-buffer bury-or-kill) + (if (switch-to-prev-buffer-skip-p skip window new-buffer bury-or-kill) (setq skipped new-buffer) (set-window-buffer-start-and-point window new-buffer (nth 1 entry) (nth 2 entry)) @@ -4740,11 +4780,7 @@ switch-to-prev-buffer ;; Scan reverted next buffers last (must not use nreverse ;; here!). (dolist (buffer (reverse next-buffers)) - ;; Actually, buffer _must_ be live here since otherwise it - ;; would have been caught in the scan of previous buffers. - (when (and (or (buffer-live-p buffer) - (not (setq killed-buffers - (cons buffer killed-buffers)))) + (when (and (buffer-live-p buffer) (not (eq buffer old-buffer)) (or (null pred) (funcall pred buffer)) (setq entry (assq buffer (window-prev-buffers window)))) @@ -4765,9 +4801,7 @@ switch-to-prev-buffer (assq old-buffer (window-prev-buffers window))))) ;; Remove `old-buffer' from WINDOW's previous and (restored list ;; of) next buffers. - (set-window-prev-buffers - window (assq-delete-all old-buffer (window-prev-buffers window))) - (set-window-next-buffers window (delq old-buffer next-buffers)) + (unrecord-window-buffer window old-buffer) (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 @@ -4780,14 +4814,6 @@ switch-to-prev-buffer (set-window-next-buffers window (cons old-buffer (delq old-buffer next-buffers)))) - ;; Remove killed buffers from WINDOW's previous and next buffers. - (when killed-buffers - (dolist (buffer killed-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))))) - ;; Return new-buffer. new-buffer)) @@ -4819,7 +4845,7 @@ switch-to-next-buffer ((or switch-to-prev-buffer-skip (not switch-to-visible-buffer)) frame))) - new-buffer entry killed-buffers skipped) + new-buffer entry skipped) (when (window-minibuffer-p window) ;; Don't switch in minibuffer window. (unless (setq window (minibuffer-selected-window)) @@ -4832,9 +4858,7 @@ switch-to-next-buffer (catch 'found ;; Scan WINDOW's next buffers first. (dolist (buffer next-buffers) - (when (and (or (buffer-live-p buffer) - (not (setq killed-buffers - (cons buffer killed-buffers)))) + (when (and (buffer-live-p buffer) (not (eq buffer old-buffer)) (or (null pred) (funcall pred buffer)) (setq entry (assq buffer (window-prev-buffers window)))) @@ -4867,9 +4891,7 @@ switch-to-next-buffer (when (and (not (eq new-buffer (car entry))) (not (eq old-buffer (car entry))) (setq new-buffer (car entry)) - (or (buffer-live-p new-buffer) - (not (setq killed-buffers - (cons new-buffer killed-buffers)))) + (buffer-live-p new-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)) @@ -4885,14 +4907,6 @@ switch-to-next-buffer ;; Remove `new-buffer' from and restore WINDOW's next buffers. (set-window-next-buffers window (delq new-buffer next-buffers)) - ;; Remove killed buffers from WINDOW's previous and next buffers. - (when killed-buffers - (dolist (buffer killed-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))))) - ;; Return new-buffer. new-buffer)) @@ -5044,6 +5058,18 @@ previous-buffer (not (or executing-kbd-macro noninteractive))) (user-error "No previous buffer")))))) +(defcustom kill-buffer-quit-windows nil + "Non-nil means killing buffers shall quit windows. +If this is nil, killing a buffer may only delete windows dedicated to +that buffer. Otherwise, `kill-buffer' has `quit-restore-window' deal +with any window showing the buffer to be killed. That function may +delete such a window even if it's not dedicated to its buffer. Also, +`delete-windows-on' will use `quit-restore-window' as fallback when a +window cannot be deleted otherwise." + :type 'boolean + :version "31.1" + :group 'windows) + (defun delete-windows-on (&optional buffer-or-name frame) "Delete all windows showing BUFFER-OR-NAME. BUFFER-OR-NAME may be a buffer or the name of an existing buffer @@ -5075,21 +5101,22 @@ 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, +is dedicated to that buffer, 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 either `quit-restore-window' (provided +`kill-buffer-quit-windows' is non-nil) or `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 and remove any `quit-restore' and `quit-restore-prev' +parameters naming it." (interactive (let ((frame (cond ((and (numberp current-prefix-arg) @@ -5107,11 +5134,12 @@ 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)) + ;; Don't run 'window-deletable-functions'. + (let ((deletable (window-deletable-p window t)) (dedicated (window-dedicated-p window))) (cond ((and (eq deletable 'frame) dedicated) @@ -5120,43 +5148,77 @@ delete-windows-on ((eq deletable t) ;; Delete window. (delete-window window)) + (kill-buffer-quit-windows + (quit-restore-window window 'bury) + (when (window-live-p window) + ;; Unrecord BUFFER in this window. + (unrecord-window-buffer window buffer t))) (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))))) + ;; Restore the dedicated 'side' flag. + (when (eq dedicated 'side) + (set-window-dedicated-p window 'side)) + (when (window-live-p window) + ;; Unrecord BUFFER in this window. + (unrecord-window-buffer window buffer t))))) ;; If a window doesn't show BUFFER, unrecord BUFFER in it. - (unrecord-window-buffer window buffer))))) + (unrecord-window-buffer window buffer t))))) (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. - -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 +BUFFER-OR-NAME may be a buffer or the name of an existing buffer and +defaults to the current buffer. Minibuffer windows are not considered. + +If the option `kill-buffer-quit-windows' is nil, behave as follows: With +the exception of side windows, when a window showing BUFFER-OR-NAME is +dedicated, delete that window. If that window is the only window on its +frame, delete its frame when there are other frames left. In any other +case, call `switch-to-prev-buffer' to display some other buffer in that window. -This function removes the buffer denoted by BUFFER-OR-NAME from -all window-local buffer lists." +If `kill-buffer-quit-windows' is non-nil, call `quit-restore-window' for +any window showing BUFFER-OR-NAME with the argument BURY-OR-KILL set to +`killing' to avoid that the latter kills the buffer prematurely. + +In either case, remove the buffer denoted by BUFFER-OR-NAME from the +lists of previous and next buffers of all windows and remove any +`quit-restore' or `quit-restore-prev' parameters mentioning it. + +If, for any window showing BUFFER-OR-NAME running the abnormal hook +`window-deletable-functions' returns nil, do not delete that window but +show some other buffer in that window. + +This function is called by `kill-buffer' which kills the buffer +specified by `buffer-or-name' afterwards. It never kills a buffer by +itself." (interactive "bBuffer to replace: ") (let ((buffer (window-normalize-buffer buffer-or-name))) + ;; Scan all windows. We have to unrecord BUFFER-OR-NAME 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) + (when (eq (window-buffer window) buffer) + (if kill-buffer-quit-windows + (quit-restore-window window 'killing) + (let ((dedicated-side (eq (window-dedicated-p window) 'side))) + (when (or dedicated-side (not (window--delete window t 'kill))) + ;; 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))))) + (window--delete window nil 'kill)))))) + + (when (window-live-p window) + ;; If the fourth elements of the 'quit-restore' or + ;; 'quit-restore-prev' parameters equal BUFFER, these + ;; parameters become useless - in 'quit-restore-window' the + ;; fourth element must equal the buffer of WINDOW in order to + ;; use that parameter. If BUFFER is mentioned in the second + ;; element of the parameter, 'quit-restore-window' cannot + ;; possibly show BUFFER instead; so this parameter becomes + ;; useless too. + (unrecord-window-buffer window buffer t))))) (defcustom quit-window-hook nil "Hook run before performing any other actions in the `quit-window' command." @@ -5164,6 +5226,23 @@ quit-window-hook :version "27.1" :group 'windows) +(defcustom quit-restore-window-no-switch nil + "Non-nil means `quit-restore-window' preferably won't switch buffers. +If this is nil, `quit-restore-window' unconditionally calls +`switch-to-prev-buffer' unless the window is dedicated or has been made +by `display-buffer'. If this is t, `quit-restore-window' will try to +delete the window unless a live buffer exists that was previously shown +in that window. If this is the symbol `skip-first', it will switch to a +previous buffer only if there are at least two of them. + +The net effect of making this non-nil is that if `quit-restore-window' +doesn't find a suitable buffer previously shown in the window, it will +rather try to delete the window (and maybe its frame) than show a buffer +the window has never shown before." + :type 'boolean + :version "31.1" + :group 'windows) + (defun window--quit-restore-select-window (window) "Select WINDOW after having quit another one. Do not select an inactive minibuffer window." @@ -5176,17 +5255,21 @@ 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' 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 -deleted. +If WINDOW's dedicated flag is t, try to delete WINDOW. If it equals the +value `side', restore that value when WINDOW is not deleted. Whether +WINDOW or its frame get deleted can be further controlled via the option +`quit-restore-window-no-switch'. + +If running the abnormal hook `window-deletable-functions' returns nil, +do not delete WINDOW but show some other buffer in it. Optional second argument BURY-OR-KILL tells how to proceed with the buffer of WINDOW. The following values are handled: @@ -5206,21 +5289,31 @@ quit-restore-window most reliable remedy to not have `switch-to-prev-buffer' switch to this buffer again without killing the buffer. -`kill' means to kill WINDOW's buffer." +`kill' means to kill WINDOW's buffer. + +`killing' is like `kill' but means that WINDOW's buffer will get killed +elsewhere. This value is used by `replace-buffer-in-windows' and +`quit-windows-on'. + +`burying' is like `bury' but means that WINDOW's buffer will get buried +elsewhere. This value is used by `quit-windows-on'." (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)) - (window--delete window 'dedicated (eq bury-or-kill 'kill))) + (window--delete + window 'dedicated (memq bury-or-kill '(kill killing)))) ;; If the previously selected window is still alive, select it. (window--quit-restore-select-window quit-restore-2)) ((and (not prev-buffer) @@ -5241,10 +5334,27 @@ quit-restore-window (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. + ((and (or (and quit-restore-window-no-switch (not prev-buffer)) + ;; Ignore first of the previous buffers if + ;; 'quit-restore-window-no-switch' says so. + (and (eq quit-restore-window-no-switch 'skip-first) + (not (cdr (window-prev-buffers window))))) + ;; Delete WINDOW if possible. + (window--delete + window nil (memq bury-or-kill '(kill killing)))) + ;; If the previously selected window is still alive, select it. + (window--quit-restore-select-window quit-restore-2)) + ((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)) @@ -5269,27 +5379,26 @@ quit-restore-window ;; 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 + (when (memq bury-or-kill '(bury burying kill killing)) ;; 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)))) + (unrecord-window-buffer window buffer)) (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. + ;; 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)) ;; 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,16 +5406,14 @@ 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 (memq bury-or-kill '(kill killing)))))) ;; Deal with the buffer. (cond ((not (buffer-live-p buffer))) ((eq bury-or-kill 'kill) (kill-buffer buffer)) - (bury-or-kill + ((eq bury-or-kill 'bury) (bury-buffer-internal buffer))))) (defun quit-window (&optional kill window) @@ -5336,18 +5443,31 @@ 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. - (unrecord-window-buffer window buffer))))) + ;; `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) + (with-current-buffer buffer + (run-hooks 'quit-window-hook)) + (quit-restore-window + window (if kill 'killing 'burying))) + + (when (window-live-p window) + ;; Unrecord BUFFER in this window. + (unrecord-window-buffer window buffer t))) + + ;; Deal with BUFFER-OR-NAME. + (cond + ((not (buffer-live-p buffer))) + (kill (kill-buffer buffer)) + (t (bury-buffer-internal buffer))))) \f (defun window--combination-resizable (parent &optional horizontal) "Return number of pixels recoverable from height of window PARENT. @@ -6657,34 +6777,42 @@ 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. This allows for +quitting WINDOW in a similar fashion but also keeps the very first +`quit-restore' parameter stored for this window around. Consequently, +WINDOW (or its frame) can be eventually deleted by `quit-restore-widow' +if that parameter's fourth element equals WINDOW's buffer." (cond ((eq type 'reuse) - (if (eq (window-buffer window) buffer) - ;; WINDOW shows BUFFER already. Update WINDOW's quit-restore - ;; parameter, if any. - (let ((quit-restore (window-parameter window 'quit-restore))) + (let ((quit-restore (window-parameter window 'quit-restore))) + (if (eq (window-buffer window) buffer) + ;; WINDOW shows BUFFER already. Update WINDOW's quit-restore + ;; parameter, if any. (when (consp quit-restore) (setcar quit-restore 'same) ;; The selected-window might have changed in ;; between (Bug#20353). (unless (or (eq window (selected-window)) - (eq window (nth 2 quit-restore))) - (setcar (cddr quit-restore) (selected-window))))) - ;; WINDOW shows another buffer. - (with-current-buffer (window-buffer window) - (set-window-parameter - window 'quit-restore - (list 'other - ;; A quadruple of WINDOW's buffer, start, point and height. - (list (current-buffer) (window-start window) - ;; Preserve window-point-insertion-type (Bug#12855). - (copy-marker - (window-point window) window-point-insertion-type) - (if (window-combined-p window) - (window-total-height window) - (window-total-width window))) - (selected-window) buffer))))) + (eq window (nth 2 quit-restore))) + (setcar (cddr quit-restore) (selected-window)))) + ;; WINDOW shows another buffer. + (with-current-buffer (window-buffer window) + (set-window-parameter + window (if quit-restore 'quit-restore-prev 'quit-restore) + (list 'other + ;; A quadruple of WINDOW's buffer, start, point and height. + (list (current-buffer) (window-start window) + ;; Preserve window-point-insertion-type (Bug#12855). + (copy-marker + (window-point window) window-point-insertion-type) + (if (window-combined-p window) + (window-total-height window) + (window-total-width window))) + (selected-window) buffer)))))) ((eq type 'window) ;; WINDOW has been created on an existing frame. (set-window-parameter @@ -9129,6 +9257,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 @@ -9189,6 +9320,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 diff --git a/src/alloc.c b/src/alloc.c index 48b170b866f..06fe12cff3d 100644 --- a/src/alloc.c +++ b/src/alloc.c @@ -7009,33 +7009,6 @@ mark_face_cache (struct face_cache *c) } } -/* Remove killed buffers or items whose car is a killed buffer from - LIST, and mark other items. Return changed LIST, which is marked. */ - -static Lisp_Object -mark_discard_killed_buffers (Lisp_Object list) -{ - Lisp_Object tail, *prev = &list; - - for (tail = list; CONSP (tail) && !cons_marked_p (XCONS (tail)); - tail = XCDR (tail)) - { - Lisp_Object tem = XCAR (tail); - if (CONSP (tem)) - tem = XCAR (tem); - if (BUFFERP (tem) && !BUFFER_LIVE_P (XBUFFER (tem))) - *prev = XCDR (tail); - else - { - set_cons_marked (XCONS (tail)); - mark_object (XCAR (tail)); - prev = xcdr_addr (tail); - } - } - mark_object (tail); - return list; -} - static void mark_frame (struct Lisp_Vector *ptr) { @@ -7090,15 +7063,6 @@ mark_window (struct Lisp_Vector *ptr) mark_glyph_matrix (w->current_matrix); mark_glyph_matrix (w->desired_matrix); } - - /* Filter out killed buffers from both buffer lists - in attempt to help GC to reclaim killed buffers faster. - We can do it elsewhere for live windows, but this is the - best place to do it for dead windows. */ - wset_prev_buffers - (w, mark_discard_killed_buffers (w->prev_buffers)); - wset_next_buffers - (w, mark_discard_killed_buffers (w->next_buffers)); } /* Entry of the mark stack. */ diff --git a/src/buffer.c b/src/buffer.c index 744b0ef5548..6ec40aff646 100644 --- a/src/buffer.c +++ b/src/buffer.c @@ -2012,6 +2012,13 @@ DEFUN ("kill-buffer", Fkill_buffer, Skill_buffer, 0, 1, "bKill buffer: ", buffer (bug#10114). */ replace_buffer_in_windows (buffer); + /* For dead windows that have not been collected yet, remove this + buffer from those windows' lists of previously and next shown + buffers and remove any 'quit-restore' or 'quit-restore-prev' + parameters mentioning the buffer. */ + if (XFIXNUM (BVAR (b, display_count)) > 0) + window_discard_buffer_from_dead_windows (buffer); + /* Exit if replacing the buffer in windows has killed our buffer. */ if (!BUFFER_LIVE_P (b)) return Qt; diff --git a/src/window.c b/src/window.c index 4bb36b6733a..559919689a3 100644 --- a/src/window.c +++ b/src/window.c @@ -3277,6 +3277,113 @@ window_pixel_to_total (Lisp_Object frame, Lisp_Object horizontal) } +/** Remove all occurrences of element whose car is BUFFER from ALIST. + Return changed ALIST. */ +static Lisp_Object +window_discard_buffer_from_alist (Lisp_Object buffer, Lisp_Object alist) +{ + Lisp_Object tail, *prev = &alist; + + for (tail = alist; CONSP (tail); tail = XCDR (tail)) + { + Lisp_Object tem = XCAR (tail); + + tem = XCAR (tem); + + if (EQ (tem, buffer)) + *prev = XCDR (tail); + else + prev = xcdr_addr (tail); + } + + return alist; +} + +/** Remove all occurrences of BUFFER from LIST. Return changed + LIST. */ +static Lisp_Object +window_discard_buffer_from_list (Lisp_Object buffer, Lisp_Object list) +{ + Lisp_Object tail, *prev = &list; + + for (tail = list; CONSP (tail); tail = XCDR (tail)) + if (EQ (XCAR (tail), buffer)) + *prev = XCDR (tail); + else + prev = xcdr_addr (tail); + + return list; +} + +/** Remove BUFFER from the lists of previous and next buffers of object + WINDOW. ALL true means remove any `quit-restore' and + `quit-restore-prev' parameter of WINDOW referencing BUFFER too. */ +static void +window_discard_buffer_from_window (Lisp_Object buffer, Lisp_Object window, bool all) +{ + struct window *w = XWINDOW (window); + + wset_prev_buffers + (w, window_discard_buffer_from_alist (buffer, w->prev_buffers)); + wset_next_buffers + (w, window_discard_buffer_from_list (buffer, w->next_buffers)); + + if (all) + { + Lisp_Object quit_restore = window_parameter (w, Qquit_restore); + Lisp_Object quit_restore_prev = window_parameter (w, Qquit_restore_prev); + Lisp_Object quad; + + if (EQ (buffer, Fnth (make_fixnum (3), quit_restore_prev)) + || (CONSP (quad = Fcar (Fcdr (quit_restore_prev))) + && EQ (Fcar (quad), buffer))) + Fset_window_parameter (window, Qquit_restore_prev, Qnil); + + if (EQ (buffer, Fnth (make_fixnum (3), quit_restore)) + || (CONSP (quad = Fcar (Fcdr (quit_restore))) + && EQ (Fcar (quad), buffer))) + { + Fset_window_parameter (window, Qquit_restore, + window_parameter (w, Qquit_restore_prev)); + Fset_window_parameter (window, Qquit_restore_prev, Qnil); + } + } +} + +/** Remove BUFFER from the lists of previous and next buffers and the + `quit-restore' and `quit-restore-prev' parameters of any dead + WINDOW. */ +void +window_discard_buffer_from_dead_windows (Lisp_Object buffer) +{ + struct Lisp_Hash_Table *h = XHASH_TABLE (window_dead_windows_table); + + DOHASH (h, k, v) + window_discard_buffer_from_window (buffer, v, true); +} + +DEFUN ("window-discard-buffer-from-window", Fwindow_discard_buffer, + Swindow_discard_buffer, 2, 3, 0, + doc: /* Discard BUFFER from WINDOW. +Discard specified live BUFFER from the lists of previous and next +buffers of specified live WINDOW. + +Optional argument ALL non-nil means discard any `quit-restore' and +`quit-restore-prev' parameters of WINDOW referencing BUFFER too. */) + (Lisp_Object buffer, Lisp_Object window, Lisp_Object all) +{ + if (!BUFFER_LIVE_P (XBUFFER (buffer))) + error ("Not a live buffer"); + + if (!WINDOW_LIVE_P (window)) + error ("Not a live window"); + + window_discard_buffer_from_window (buffer, window, !NILP (all)); + + return Qnil; +} + + DEFUN ("delete-other-windows-internal", Fdelete_other_windows_internal, Sdelete_other_windows_internal, 0, 2, "", doc: /* Make WINDOW fill its frame. @@ -4140,6 +4247,9 @@ set_window_buffer (Lisp_Object window, Lisp_Object buffer, w->window_end_vpos = 0; w->last_cursor_vpos = 0; + /* Discard BUFFER from WINDOW's previous and next buffers. */ + window_discard_buffer_from_window (buffer, window, false); + if (!(keep_margins_p && samebuf)) { /* If we're not actually changing the buffer, don't reset hscroll and vscroll. Resetting hscroll and vscroll here is problematic @@ -4402,6 +4512,10 @@ make_parent_window (Lisp_Object window, bool horflag) wset_buffer (p, Qnil); wset_combination (p, horflag, window); wset_combination_limit (p, Qnil); + /* Reset any previous and next buffers of p which have been installed + by the memcpy above. */ + wset_prev_buffers (p, Qnil); + wset_next_buffers (p, Qnil); wset_window_parameters (p, Qnil); } @@ -4426,10 +4540,6 @@ make_window (void) wset_vertical_scroll_bar_type (w, Qt); wset_horizontal_scroll_bar_type (w, Qt); wset_cursor_type (w, Qt); - /* These Lisp fields are marked specially so they're not set to nil by - allocate_window. */ - wset_prev_buffers (w, Qnil); - wset_next_buffers (w, Qnil); /* Initialize non-Lisp data. Note that allocate_window zeroes out all non-Lisp data, so do it only for slots which should not be zero. */ @@ -5252,6 +5362,11 @@ DEFUN ("delete-window-internal", Fdelete_window_internal, Sdelete_window_interna unchain_marker (XMARKER (w->old_pointm)); unchain_marker (XMARKER (w->start)); wset_buffer (w, Qnil); + /* Add WINDOW to table of dead windows so when killing a buffer + WINDOW mentions, all references to that buffer can be removed + and the buffer be collected. */ + Fputhash (make_fixnum (w->sequence_number), + window, window_dead_windows_table); } if (NILP (s->prev) && NILP (s->next)) @@ -7356,12 +7471,21 @@ DEFUN ("set-window-configuration", Fset_window_configuration, } } + /* Remove window from the table of dead windows. */ + Fremhash (make_fixnum (w->sequence_number), + window_dead_windows_table); + if ((NILP (dont_set_miniwindow) || !MINI_WINDOW_P (w)) && BUFFERP (p->buffer) && BUFFER_LIVE_P (XBUFFER (p->buffer))) /* If saved buffer is alive, install it, unless it's a minibuffer we explicitly prohibit. */ { - wset_buffer (w, p->buffer); + if (!EQ (w->contents, p->buffer)) + { + wset_buffer (w, p->buffer); + window_discard_buffer_from_window (w->contents, window, false); + } + w->start_at_line_beg = !NILP (p->start_at_line_beg); set_marker_restricted (w->start, p->start, w->contents); set_marker_restricted (w->pointm, p->pointm, w->contents); @@ -7406,6 +7530,7 @@ DEFUN ("set-window-configuration", Fset_window_configuration, recreate *scratch* in the course (part of Juanma's bs-show scenario from March 2011). */ wset_buffer (w, other_buffer_safely (Fcurrent_buffer ())); + window_discard_buffer_from_window (w->contents, window, false); /* This will set the markers to beginning of visible range. */ set_marker_restricted_both (w->start, w->contents, 0, 0); @@ -7585,6 +7710,11 @@ delete_all_child_windows (Lisp_Object window) possible resurrection in Fset_window_configuration. */ wset_combination_limit (w, w->contents); wset_buffer (w, Qnil); + /* Add WINDOW to table of dead windows so when killing a buffer + WINDOW mentions, all references to that buffer can be removed + and the buffer be collected. */ + Fputhash (make_fixnum (w->sequence_number), + window, window_dead_windows_table); } Vwindow_list = Qnil; @@ -8594,6 +8724,8 @@ syms_of_window (void) DEFSYM (Qconfiguration, "configuration"); DEFSYM (Qdelete, "delete"); DEFSYM (Qdedicated, "dedicated"); + DEFSYM (Qquit_restore, "quit-restore"); + DEFSYM (Qquit_restore_prev, "quit-restore-prev"); DEFVAR_LISP ("temp-buffer-show-function", Vtemp_buffer_show_function, doc: /* Non-nil means call as function to display a help buffer. @@ -8917,6 +9049,17 @@ syms_of_window (void) displayed after a scrolling operation to be somewhat inaccurate. */); fast_but_imprecise_scrolling = false; + DEFVAR_LISP ("window-dead-windows-table", window_dead_windows_table, + doc: /* Hash table of dead windows. +Each entry in this table maps a window number to a window object. +Entries are added by `delete-window-internal' and are removed by the +garbage collector. + +This table is maintained by code in window.c and is made visible in +Elisp for testing purposes only. */); + window_dead_windows_table + = CALLN (Fmake_hash_table, QCweakness, Qt); + defsubr (&Sselected_window); defsubr (&Sold_selected_window); defsubr (&Sminibuffer_window); @@ -9032,6 +9175,7 @@ syms_of_window (void) defsubr (&Swindow_parameters); defsubr (&Swindow_parameter); defsubr (&Sset_window_parameter); + defsubr (&Swindow_discard_buffer); defsubr (&Swindow_cursor_type); defsubr (&Sset_window_cursor_type); } diff --git a/src/window.h b/src/window.h index 86932181252..335e0a3453e 100644 --- a/src/window.h +++ b/src/window.h @@ -142,6 +142,12 @@ #define WINDOW_H_INCLUDED as well. */ Lisp_Object contents; + /* A list of <buffer, window-start, window-point> triples listing + buffers previously shown in this window. */ + Lisp_Object prev_buffers; + /* List of buffers re-shown in this window. */ + Lisp_Object next_buffers; + /* The old buffer of this window, set to this window's buffer by run_window_change_functions every time it sees this window. Unused for internal windows. */ @@ -218,14 +224,6 @@ #define WINDOW_H_INCLUDED struct glyph_matrix *current_matrix; struct glyph_matrix *desired_matrix; - /* The two Lisp_Object fields below are marked in a special way, - which is why they're placed after `current_matrix'. */ - /* A list of <buffer, window-start, window-point> triples listing - buffers previously shown in this window. */ - Lisp_Object prev_buffers; - /* List of buffers re-shown in this window. */ - Lisp_Object next_buffers; - /* Number saying how recently window was selected. */ EMACS_INT use_time; @@ -1228,6 +1226,7 @@ #define CHECK_LIVE_WINDOW(WINDOW) \ extern void wset_buffer (struct window *, Lisp_Object); extern bool window_outdated (struct window *); extern ptrdiff_t window_point (struct window *w); +extern void window_discard_buffer_from_dead_windows (Lisp_Object); extern void init_window_once (void); extern void init_window (void); extern void syms_of_window (void); ^ permalink raw reply related [flat|nested] 45+ messages in thread
* bug#59862: quit-restore per window buffer 2024-07-30 8:20 ` martin rudalics via Bug reports for GNU Emacs, the Swiss army knife of text editors @ 2024-07-31 17:30 ` Juri Linkov 2024-08-01 6:37 ` Juri Linkov 2024-08-01 7:49 ` martin rudalics via Bug reports for GNU Emacs, the Swiss army knife of text editors 0 siblings, 2 replies; 45+ messages in thread From: Juri Linkov @ 2024-07-31 17:30 UTC (permalink / raw) To: martin rudalics; +Cc: 59862 > After fixing a few other inconsistencies find my final version of the > patch here. The ChangeLog is below. If you see no further problems, > I'll install it and you can add your changes on top of it. I've tested your previous changes for a long time, and everything works nicely. I hope your new patch is even better. However, since it grew to an unmanageable size, I will test it only after you will push it. > +*** New hook 'window-deletable-functions'. > +This abnormal hook gives its client a way to save a window from getting > +deleted implicitly by functions like 'kill-buffer', 'bury-buffer' and > +'quit-restore-window', Maybe this hook is needed for bug#71386, but I have no plans to use it for tab handling. > +*** New option 'quit-restore-window-no-switch'. > +With this option set, 'quit-restore-window' will delete its window more > +aggressively rather than switching to some other buffer in it. This option looks useful for some cases. ^ permalink raw reply [flat|nested] 45+ messages in thread
* bug#59862: quit-restore per window buffer 2024-07-31 17:30 ` Juri Linkov @ 2024-08-01 6:37 ` Juri Linkov 2024-08-01 7:50 ` martin rudalics via Bug reports for GNU Emacs, the Swiss army knife of text editors 2024-08-01 7:49 ` martin rudalics via Bug reports for GNU Emacs, the Swiss army knife of text editors 1 sibling, 1 reply; 45+ messages in thread From: Juri Linkov @ 2024-08-01 6:37 UTC (permalink / raw) To: martin rudalics; +Cc: 59862 >> After fixing a few other inconsistencies find my final version of the >> patch here. The ChangeLog is below. If you see no further problems, >> I'll install it and you can add your changes on top of it. > > I've tested your previous changes for a long time, > and everything works nicely. I hope your new patch is even > better. However, since it grew to an unmanageable size, > I will test it only after you will push it. > >> +*** New hook 'window-deletable-functions'. >> +This abnormal hook gives its client a way to save a window from getting >> +deleted implicitly by functions like 'kill-buffer', 'bury-buffer' and >> +'quit-restore-window', > > Maybe this hook is needed for bug#71386, but I have no plans > to use it for tab handling. Or maybe will use it for tabs, I don't know, need to try, but not sure if you expect such try before you push or after. ^ permalink raw reply [flat|nested] 45+ messages in thread
* bug#59862: quit-restore per window buffer 2024-08-01 6:37 ` Juri Linkov @ 2024-08-01 7:50 ` martin rudalics via Bug reports for GNU Emacs, the Swiss army knife of text editors 0 siblings, 0 replies; 45+ messages in thread From: martin rudalics via Bug reports for GNU Emacs, the Swiss army knife of text editors @ 2024-08-01 7:50 UTC (permalink / raw) To: Juri Linkov; +Cc: 59862 > Or maybe will use it for tabs, I don't know, need to try, > but not sure if you expect such try before you push or after. After, by now. martin ^ permalink raw reply [flat|nested] 45+ messages in thread
* bug#59862: quit-restore per window buffer 2024-07-31 17:30 ` Juri Linkov 2024-08-01 6:37 ` Juri Linkov @ 2024-08-01 7:49 ` martin rudalics via Bug reports for GNU Emacs, the Swiss army knife of text editors 2024-08-01 16:01 ` Juri Linkov 1 sibling, 1 reply; 45+ messages in thread From: martin rudalics via Bug reports for GNU Emacs, the Swiss army knife of text editors @ 2024-08-01 7:49 UTC (permalink / raw) To: Juri Linkov; +Cc: 59862 > I've tested your previous changes for a long time, > and everything works nicely. I hope your new patch is even > better. However, since it grew to an unmanageable size, > I will test it only after you will push it. Pushed now. Lets hope for the best. Please consider closing Bug#59862. It has become unmanageable for me as well. >> +*** New hook 'window-deletable-functions'. >> +This abnormal hook gives its client a way to save a window from getting >> +deleted implicitly by functions like 'kill-buffer', 'bury-buffer' and >> +'quit-restore-window', > > Maybe this hook is needed for bug#71386, but I have no plans > to use it for tab handling. Actually, this is the opposite of ... >> +*** New option 'quit-restore-window-no-switch'. >> +With this option set, 'quit-restore-window' will delete its window more >> +aggressively rather than switching to some other buffer in it. > > This option looks useful for some cases. ... that. So people may decide which of them suits their needs better. martin ^ permalink raw reply [flat|nested] 45+ messages in thread
* bug#59862: quit-restore per window buffer 2024-08-01 7:49 ` martin rudalics via Bug reports for GNU Emacs, the Swiss army knife of text editors @ 2024-08-01 16:01 ` Juri Linkov 2024-10-08 17:53 ` Juri Linkov 0 siblings, 1 reply; 45+ messages in thread From: Juri Linkov @ 2024-08-01 16:01 UTC (permalink / raw) To: martin rudalics; +Cc: 59862 >> I've tested your previous changes for a long time, >> and everything works nicely. I hope your new patch is even >> better. However, since it grew to an unmanageable size, >> I will test it only after you will push it. > > Pushed now. Lets hope for the best. Please consider closing Bug#59862. Thank you! I'm going to test bug#59862 and bug#71386. ^ permalink raw reply [flat|nested] 45+ messages in thread
* bug#59862: quit-restore per window buffer 2024-08-01 16:01 ` Juri Linkov @ 2024-10-08 17:53 ` Juri Linkov 2024-10-09 8:46 ` martin rudalics via Bug reports for GNU Emacs, the Swiss army knife of text editors 0 siblings, 1 reply; 45+ messages in thread From: Juri Linkov @ 2024-10-08 17:53 UTC (permalink / raw) To: martin rudalics; +Cc: 59862 >> Pushed now. Lets hope for the best. Please consider closing Bug#59862. > > Thank you! I'm going to test bug#59862 and bug#71386. I tried to support this case C-x t t ;; other-tab-prefix C-h i ;; info C-h e ;; view-echo-area-messages q ;; quit-window should not close the tab using the new hook 'window-deletable-functions', but I see no way. Could you please help. Here is what I did: diff --git a/lisp/window.el b/lisp/window.el index e08680aa392..5b4f642afc9 100644 --- a/lisp/window.el +++ b/lisp/window.el @@ -5340,13 +5346,7 @@ 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) - (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. - (window--quit-restore-select-window quit-restore-2)) - ((and (not prev-buffer) - (or (eq (nth 1 quit-restore) 'frame) + (or (memq (nth 1 quit-restore) '(frame tab)) (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 So frame and tab will be handled equally. Then also code for tab-bar.el: (defun tab-bar-window-deletable (window single) (if (and single (eq (nth 1 (window-parameter window 'quit-restore)) 'tab)) (progn (tab-bar-close-tab) nil) t)) (add-hook 'window-deletable-functions 'tab-bar-window-deletable) But 'window-deletable-functions' is not called at all, because 'window-deletable-p' first checks (frame-deletable-p (window-frame window)) and a single frame with tabs is not deletable. Also instead of 'frame-root-window-p' in 'window-deletable-p' the tab-bar needs to use another code. ^ permalink raw reply related [flat|nested] 45+ messages in thread
* bug#59862: quit-restore per window buffer 2024-10-08 17:53 ` Juri Linkov @ 2024-10-09 8:46 ` martin rudalics via Bug reports for GNU Emacs, the Swiss army knife of text editors 2024-10-09 16:17 ` Juri Linkov 0 siblings, 1 reply; 45+ messages in thread From: martin rudalics via Bug reports for GNU Emacs, the Swiss army knife of text editors @ 2024-10-09 8:46 UTC (permalink / raw) To: Juri Linkov; +Cc: 59862 > > (defun tab-bar-window-deletable (window single) > (if (and single (eq (nth 1 (window-parameter window 'quit-restore)) 'tab)) > (progn (tab-bar-close-tab) nil) > t)) > > (add-hook 'window-deletable-functions 'tab-bar-window-deletable) > > But 'window-deletable-functions' is not called at all, > because 'window-deletable-p' first checks > > (frame-deletable-p (window-frame window)) > > and a single frame with tabs is not deletable. Why "with tabs" and not just "a single frame is not deletable"? > Also instead of 'frame-root-window-p' in 'window-deletable-p' > the tab-bar needs to use another code. I don't understand the problem yet. 'window-deletable-p' calls 'frame-deletable-p' iff WINDOW is the root window of the frame. At this time we can only either delete the frame or show another buffer in WINDOW; tertium non datur. What am I missing? martin ^ permalink raw reply [flat|nested] 45+ messages in thread
* bug#59862: quit-restore per window buffer 2024-10-09 8:46 ` martin rudalics via Bug reports for GNU Emacs, the Swiss army knife of text editors @ 2024-10-09 16:17 ` Juri Linkov 2024-10-10 14:54 ` martin rudalics via Bug reports for GNU Emacs, the Swiss army knife of text editors 0 siblings, 1 reply; 45+ messages in thread From: Juri Linkov @ 2024-10-09 16:17 UTC (permalink / raw) To: martin rudalics; +Cc: 59862 >> >> (defun tab-bar-window-deletable (window single) >> (if (and single (eq (nth 1 (window-parameter window 'quit-restore)) 'tab)) >> (progn (tab-bar-close-tab) nil) >> t)) >> >> (add-hook 'window-deletable-functions 'tab-bar-window-deletable) >> >> But 'window-deletable-functions' is not called at all, >> because 'window-deletable-p' first checks >> >> (frame-deletable-p (window-frame window)) >> >> and a single frame with tabs is not deletable. > > Why "with tabs" and not just "a single frame is not deletable"? Because tabs have priority over frames. The precedence is: windows -> tabs -> frames. When there are many tabs then quit-restore-window should close tabs, and only on the last tab should delete the frame. This is bug#71386, but first need to finish bug#59862. >> Also instead of 'frame-root-window-p' in 'window-deletable-p' >> the tab-bar needs to use another code. > > I don't understand the problem yet. 'window-deletable-p' calls > 'frame-deletable-p' iff WINDOW is the root window of the frame. At this > time we can only either delete the frame or show another buffer in > WINDOW; tertium non datur. What am I missing? The third first-class citizen is the tab-bar. Here is the test case that will help to understand the requirements. 1.1. 'quit-restore-window' deletes the frame 2.1. 'quit-restore-window' deletes the tab Both test cases are identical, and already pass correctly. 1.2. 'quit-restore-window' doesn't delete the frame passes as well, but its counterpart 2.2. 'quit-restore-window' doesn't delete the tab fails at the line "FAILS HERE": ``` diff --git a/test/lisp/tab-bar-tests.el b/test/lisp/tab-bar-tests.el index aa8384b24e8..00fd78cf081 100644 --- a/test/lisp/tab-bar-tests.el +++ b/test/lisp/tab-bar-tests.el @@ -42,10 +42,104 @@ tab-bar-tests-close-other-tabs (should (eq (length tab-bar-closed-tabs) 0))) (ert-deftest tab-bar-tests-close-other-tabs-default () - (tab-bar-tests-close-other-tabs nil)) + (tab-bar-tests-close-other-tabs nil) + ;; Clean up tabs afterwards + (tab-bar-tabs-set nil)) (ert-deftest tab-bar-tests-close-other-tabs-with-arg () - (dotimes (i 5) (tab-bar-tests-close-other-tabs i))) + (dotimes (i 5) (tab-bar-tests-close-other-tabs i)) + ;; Clean up tabs afterwards + (tab-bar-tabs-set nil)) + +(ert-deftest tab-bar-tests-quit-restore-window () + (let* ((frame-params (when noninteractive + '((window-system . nil) + (tty-type . "linux")))) + (pop-up-frame-alist frame-params) + (frame-auto-hide-function 'delete-frame)) + + ;; 1.1. 'quit-restore-window' deletes the frame + (progn + (should (eq (length (frame-list)) 1)) + (other-frame-prefix) + (info) + (should (eq (length (frame-list)) 2)) + (should (equal (buffer-name) "*info*")) + (view-echo-area-messages) + (other-window 1) + (should (eq (length (window-list)) 2)) + (should (equal (buffer-name) "*Messages*")) + (quit-window) + (should (eq (length (window-list)) 1)) + (should (equal (buffer-name) "*info*")) + (quit-window) + (should (eq (length (frame-list)) 1))) + + ;; 1.2. 'quit-restore-window' doesn't delete the frame + (progn + (should (eq (length (frame-list)) 1)) + (other-frame-prefix) + (info) + (should (eq (length (frame-list)) 2)) + (should (equal (buffer-name) "*info*")) + (view-echo-area-messages) + (should (eq (length (window-list)) 2)) + (should (equal (buffer-name) "*info*")) + (quit-window) + (should (eq (length (window-list)) 1)) + (should (eq (length (frame-list)) 2)) + (should (equal (buffer-name) "*Messages*")) + (quit-window) + (should (eq (length (frame-list)) 2)) + ;; Clean up the frame afterwards + (delete-frame)) + + ;; 2.1. 'quit-restore-window' deletes the tab + (progn + (should (eq (length (tab-bar-tabs)) 1)) + (other-tab-prefix) + (info) + (should (eq (length (tab-bar-tabs)) 2)) + (should (equal (buffer-name) "*info*")) + (view-echo-area-messages) + (other-window 1) + (should (eq (length (window-list)) 2)) + (should (equal (buffer-name) "*Messages*")) + (quit-window) + (should (eq (length (window-list)) 1)) + (should (equal (buffer-name) "*info*")) + (quit-window) + (should (eq (length (tab-bar-tabs)) 1))) + + ;; 2.2. 'quit-restore-window' doesn't delete the tab + (progn + (should (eq (length (tab-bar-tabs)) 1)) + (other-tab-prefix) + (info) + (should (eq (length (tab-bar-tabs)) 2)) + (should (equal (buffer-name) "*info*")) + (view-echo-area-messages) + (should (eq (length (window-list)) 2)) + (should (equal (buffer-name) "*info*")) + (quit-window) + (should (eq (length (window-list)) 1)) + (should (eq (length (tab-bar-tabs)) 2)) ;; FAILS HERE + (should (equal (buffer-name) "*Messages*")) + (quit-window) + (should (eq (length (tab-bar-tabs)) 2)) + ;; Clean up the tab afterwards + (tab-close)) + + ;; Clean up tabs afterwards + (tab-bar-tabs-set nil))) (provide 'tab-bar-tests) ;;; tab-bar-tests.el ends here ``` ^ permalink raw reply related [flat|nested] 45+ messages in thread
* bug#59862: quit-restore per window buffer 2024-10-09 16:17 ` Juri Linkov @ 2024-10-10 14:54 ` martin rudalics via Bug reports for GNU Emacs, the Swiss army knife of text editors 2024-10-10 18:28 ` Juri Linkov 0 siblings, 1 reply; 45+ messages in thread From: martin rudalics via Bug reports for GNU Emacs, the Swiss army knife of text editors @ 2024-10-10 14:54 UTC (permalink / raw) To: Juri Linkov; +Cc: 59862 > Because tabs have priority over frames. The precedence is: > windows -> tabs -> frames. When there are many tabs > then quit-restore-window should close tabs, and only > on the last tab should delete the frame. This is bug#71386, > but first need to finish bug#59862. So which bug are we discussing here? Bug#59862 or Bug#71386? >> I don't understand the problem yet. 'window-deletable-p' calls >> 'frame-deletable-p' iff WINDOW is the root window of the frame. At this >> time we can only either delete the frame or show another buffer in >> WINDOW; tertium non datur. What am I missing? > > The third first-class citizen is the tab-bar. But I haven't written anything for the tab-bar code. This is something you have to provide. Earlier you proposed diff --git a/lisp/window.el b/lisp/window.el index 58120c919c7..82efb3c40ce 100644 --- a/lisp/window.el +++ b/lisp/window.el @@ -4120,6 +4120,11 @@ window-deletable-p (let ((frame (window-frame window))) (cond + ((and (eq (nth 1 (window-parameter window 'quit-restore)) 'tab) + (< (seq-count (lambda (w) (window-parameter w 'quit-restore)) + (window-list-1 nil 'nomini)) + 2)) + 'tab) ((frame-root-window-p window) ;; WINDOW's frame can be deleted only if there are other frames ;; on the same terminal, and it does not contain the active @@ -4990,6 +4995,9 @@ window--delete (unless (and dedicated-only (not (window-dedicated-p window))) (let ((deletable (window-deletable-p window))) (cond + ((eq deletable 'tab) + (tab-bar-close-tab) + 'tab) ((eq deletable 'frame) (let ((frame (window-frame window))) (cond @@ -5303,10 +5311,8 @@ quit-restore-window ((and (not prev-buffer) (eq (nth 1 quit-restore) 'tab) (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) + (window--delete + window nil (memq bury-or-kill '(kill killing)))) ;; If the previously selected window is still alive, select it. (window--quit-restore-select-window quit-restore-2)) ((and (not prev-buffer) martin ^ permalink raw reply related [flat|nested] 45+ messages in thread
* bug#59862: quit-restore per window buffer 2024-10-10 14:54 ` martin rudalics via Bug reports for GNU Emacs, the Swiss army knife of text editors @ 2024-10-10 18:28 ` Juri Linkov 2024-10-11 7:50 ` martin rudalics via Bug reports for GNU Emacs, the Swiss army knife of text editors 0 siblings, 1 reply; 45+ messages in thread From: Juri Linkov @ 2024-10-10 18:28 UTC (permalink / raw) To: martin rudalics; +Cc: 59862 [-- Attachment #1: Type: text/plain, Size: 1069 bytes --] > > Because tabs have priority over frames. The precedence is: > > windows -> tabs -> frames. When there are many tabs > > then quit-restore-window should close tabs, and only > > on the last tab should delete the frame. This is bug#71386, > > but first need to finish bug#59862. > > So which bug are we discussing here? Bug#59862 or Bug#71386? I intended to finish first bug#59862, then to do bug#71386 later. But really they are closely related, so I fixed them both now and added corresponding references to bug numbers in comments. > >> I don't understand the problem yet. 'window-deletable-p' calls > >> 'frame-deletable-p' iff WINDOW is the root window of the frame. At this > >> time we can only either delete the frame or show another buffer in > >> WINDOW; tertium non datur. What am I missing? > > > > The third first-class citizen is the tab-bar. > > But I haven't written anything for the tab-bar code. This is something > you have to provide. Earlier you proposed So I extended this patch, and now all tests pass successfully with: [-- Warning: decoded text below may be mangled, UTF-8 assumed --] [-- Attachment #2: quit-restore-tab.patch --] [-- Type: text/x-diff, Size: 6537 bytes --] diff --git a/lisp/window.el b/lisp/window.el index 5822947f2fe..119de1eb38f 100644 --- a/lisp/window.el +++ b/lisp/window.el @@ -4143,6 +4143,18 @@ window-deletable-p (let ((frame (window-frame window))) (cond + ((and tab-bar-mode + ;; Fall back to frame handling in case of less than 2 tabs + (> (length (funcall tab-bar-tabs-function frame)) 1) + ;; Close the tab with the initial window (bug#59862) + (or (eq (nth 1 (window-parameter window 'quit-restore)) 'tab) + ;; or with the dedicated window (bug#71386) + (window-dedicated-p window)) + ;; Don't close the tab if more windows were created explicitly + (< (seq-count (lambda (w) (window-parameter w 'quit-restore)) + (window-list-1 nil 'nomini)) + 2)) + 'tab) ((frame-root-window-p window) ;; WINDOW's frame can be deleted only if there are other frames ;; on the same terminal, and it does not contain the active @@ -4979,6 +4991,9 @@ window--delete (unless (and dedicated-only (not (window-dedicated-p window))) (let ((deletable (window-deletable-p window))) (cond + ((eq deletable 'tab) + (tab-bar-close-tab) + 'tab) ((eq deletable 'frame) (let ((frame (window-frame window))) (cond @@ -5340,13 +5355,7 @@ 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) - (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. - (window--quit-restore-select-window quit-restore-2)) - ((and (not prev-buffer) - (or (eq (nth 1 quit-restore) 'frame) + (or (memq (nth 1 quit-restore) '(frame tab)) (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 diff --git a/test/lisp/tab-bar-tests.el b/test/lisp/tab-bar-tests.el index aa8384b24e8..8e8e75ce720 100644 --- a/test/lisp/tab-bar-tests.el +++ b/test/lisp/tab-bar-tests.el @@ -42,10 +42,116 @@ tab-bar-tests-close-other-tabs (should (eq (length tab-bar-closed-tabs) 0))) (ert-deftest tab-bar-tests-close-other-tabs-default () - (tab-bar-tests-close-other-tabs nil)) + (tab-bar-tests-close-other-tabs nil) + ;; Clean up tabs afterwards + (tab-bar-tabs-set nil)) (ert-deftest tab-bar-tests-close-other-tabs-with-arg () - (dotimes (i 5) (tab-bar-tests-close-other-tabs i))) + (dotimes (i 5) (tab-bar-tests-close-other-tabs i)) + ;; Clean up tabs afterwards + (tab-bar-tabs-set nil)) + +(ert-deftest tab-bar-tests-quit-restore-window () + (let* ((frame-params (when noninteractive + '((window-system . nil) + (tty-type . "linux")))) + (pop-up-frame-alist frame-params) + (frame-auto-hide-function 'delete-frame)) + + ;; 1.1. 'quit-restore-window' should delete the frame + ;; from initial window (bug#59862) + (progn + (should (eq (length (frame-list)) 1)) + (other-frame-prefix) + (info) + (should (eq (length (frame-list)) 2)) + (should (equal (buffer-name) "*info*")) + (view-echo-area-messages) + (other-window 1) + (should (eq (length (window-list)) 2)) + (should (equal (buffer-name) "*Messages*")) + (quit-window) + (should (eq (length (window-list)) 1)) + (should (equal (buffer-name) "*info*")) + (quit-window) + (should (eq (length (frame-list)) 1))) + + ;; 1.2. 'quit-restore-window' should not delete the frame + ;; from non-initial window (bug#59862) + (progn + (should (eq (length (frame-list)) 1)) + (other-frame-prefix) + (info) + (should (eq (length (frame-list)) 2)) + (should (equal (buffer-name) "*info*")) + (view-echo-area-messages) + (should (eq (length (window-list)) 2)) + (should (equal (buffer-name) "*info*")) + (quit-window) + (should (eq (length (window-list)) 1)) + (should (eq (length (frame-list)) 2)) + (should (equal (buffer-name) "*Messages*")) + (quit-window) + (should (eq (length (frame-list)) 2)) + ;; Clean up the frame afterwards + (delete-frame)) + + ;; 2.1. 'quit-restore-window' should close the tab + ;; from initial window (bug#59862) + (progn + (should (eq (length (tab-bar-tabs)) 1)) + (other-tab-prefix) + (info) + (should (eq (length (tab-bar-tabs)) 2)) + (should (equal (buffer-name) "*info*")) + (view-echo-area-messages) + (other-window 1) + (should (eq (length (window-list)) 2)) + (should (equal (buffer-name) "*Messages*")) + (quit-window) + (should (eq (length (window-list)) 1)) + (should (equal (buffer-name) "*info*")) + (quit-window) + (should (eq (length (tab-bar-tabs)) 1))) + + ;; 2.2. 'quit-restore-window' should not close the tab + ;; from non-initial window (bug#59862) + (progn + (should (eq (length (tab-bar-tabs)) 1)) + (other-tab-prefix) + (info) + (should (eq (length (tab-bar-tabs)) 2)) + (should (equal (buffer-name) "*info*")) + (view-echo-area-messages) + (should (eq (length (window-list)) 2)) + (should (equal (buffer-name) "*info*")) + (quit-window) + (should (eq (length (window-list)) 1)) + (should (eq (length (tab-bar-tabs)) 2)) + (should (equal (buffer-name) "*Messages*")) + (quit-window) + (should (eq (length (tab-bar-tabs)) 2)) + ;; Clean up the tab afterwards + (tab-close)) + + ;; 3. Don't delete the frame with dedicated window + ;; from the second tab (bug#71386) + (with-selected-frame (make-frame frame-params) + (switch-to-buffer (generate-new-buffer "test1")) + (tab-new) + (switch-to-buffer (generate-new-buffer "test2")) + (set-window-dedicated-p (selected-window) t) + (kill-buffer) + (should (eq (length (frame-list)) 2)) + (should (eq (length (tab-bar-tabs)) 1)) + ;; But now should delete the frame with dedicated window + ;; from the last tab + (set-window-dedicated-p (selected-window) t) + (kill-buffer) + (should (eq (length (frame-list)) 1))) + + ;; Clean up tabs afterwards + (tab-bar-tabs-set nil))) (provide 'tab-bar-tests) ;;; tab-bar-tests.el ends here ^ permalink raw reply related [flat|nested] 45+ messages in thread
* bug#59862: quit-restore per window buffer 2024-10-10 18:28 ` Juri Linkov @ 2024-10-11 7:50 ` martin rudalics via Bug reports for GNU Emacs, the Swiss army knife of text editors 2024-10-13 6:18 ` Juri Linkov 0 siblings, 1 reply; 45+ messages in thread From: martin rudalics via Bug reports for GNU Emacs, the Swiss army knife of text editors @ 2024-10-11 7:50 UTC (permalink / raw) To: Juri Linkov; +Cc: 59862 > + ((and tab-bar-mode Wouldn't (> (frame-parameter frame 'tab-bar-lines) 0) be more accurate? > + (> (length (funcall tab-bar-tabs-function frame)) 1) > + ;; Close the tab with the initial window (bug#59862) > + (or (eq (nth 1 (window-parameter window 'quit-restore)) 'tab) > + ;; or with the dedicated window (bug#71386) Please always close comments with a period. > + (window-dedicated-p window)) > + ;; Don't close the tab if more windows were created explicitly > + (< (seq-count (lambda (w) (window-parameter w 'quit-restore)) > + (window-list-1 nil 'nomini)) Shouldn't you give 'window-list-1' a FRAME argument? martin ^ permalink raw reply [flat|nested] 45+ messages in thread
* bug#59862: quit-restore per window buffer 2024-10-11 7:50 ` martin rudalics via Bug reports for GNU Emacs, the Swiss army knife of text editors @ 2024-10-13 6:18 ` Juri Linkov 0 siblings, 0 replies; 45+ messages in thread From: Juri Linkov @ 2024-10-13 6:18 UTC (permalink / raw) To: martin rudalics; +Cc: 59862 >> + ((and tab-bar-mode > > Wouldn't > > (> (frame-parameter frame 'tab-bar-lines) 0) > > be more accurate? > >> + (> (length (funcall tab-bar-tabs-function frame)) 1) >> + ;; Close the tab with the initial window (bug#59862) >> + (or (eq (nth 1 (window-parameter window 'quit-restore)) 'tab) >> + ;; or with the dedicated window (bug#71386) > > Please always close comments with a period. > >> + (window-dedicated-p window)) >> + ;; Don't close the tab if more windows were created explicitly >> + (< (seq-count (lambda (w) (window-parameter w 'quit-restore)) >> + (window-list-1 nil 'nomini)) > > Shouldn't you give 'window-list-1' a FRAME argument? Thanks, will fix these. Also noticed this needs 'car': (memq (car (window-parameter w 'quit-restore)) '(window tab frame)) Also during testing for a few days I found a problem that prevents fixing bug#71386: 1. C-x t 2 2. C-x d any_dir 3. mark files with 'm' 4. R other_dir This unexpectedly closes the tab. This is because (window-dedicated-p window) in window-deletable-p in the patch discovers that the window with the buffer " *Marked files*" is dedicated while trying to delete this window. So probably to fix bug#71386 need to add another condition that closes the tab only when there are no more windows on the frame: (and (window-dedicated-p window) (frame-root-window-p window)) ^ permalink raw reply [flat|nested] 45+ messages in thread
end of thread, other threads:[~2024-10-13 6:18 UTC | newest] Thread overview: 45+ 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 2024-07-08 16:49 ` martin rudalics via Bug reports for GNU Emacs, the Swiss army knife of text editors 2024-07-09 6:58 ` Juri Linkov 2024-07-09 8:52 ` martin rudalics via Bug reports for GNU Emacs, the Swiss army knife of text editors 2024-07-10 6:50 ` Juri Linkov 2024-07-10 9:16 ` martin rudalics via Bug reports for GNU Emacs, the Swiss army knife of text editors 2024-07-11 6:47 ` Juri Linkov 2024-07-11 8:36 ` martin rudalics via Bug reports for GNU Emacs, the Swiss army knife of text editors 2024-07-12 6:54 ` Juri Linkov 2024-07-12 8:20 ` martin rudalics via Bug reports for GNU Emacs, the Swiss army knife of text editors 2024-07-14 7:49 ` Juri Linkov 2024-07-15 7:32 ` martin rudalics via Bug reports for GNU Emacs, the Swiss army knife of text editors 2024-07-16 6:09 ` Björn Bidar via Bug reports for GNU Emacs, the Swiss army knife of text editors 2024-07-16 6:09 ` Björn Bidar via Bug reports for GNU Emacs, the Swiss army knife of text editors [not found] ` <87frs9kgve.fsf@> 2024-07-16 8:22 ` martin rudalics via Bug reports for GNU Emacs, the Swiss army knife of text editors 2024-07-16 22:14 ` Björn Bidar via Bug reports for GNU Emacs, the Swiss army knife of text editors 2024-07-16 22:14 ` Björn Bidar via Bug reports for GNU Emacs, the Swiss army knife of text editors [not found] ` <871q3tc7da.fsf@> 2024-07-17 9:23 ` martin rudalics via Bug reports for GNU Emacs, the Swiss army knife of text editors 2024-07-30 8:20 ` martin rudalics via Bug reports for GNU Emacs, the Swiss army knife of text editors 2024-07-31 17:30 ` Juri Linkov 2024-08-01 6:37 ` Juri Linkov 2024-08-01 7:50 ` martin rudalics via Bug reports for GNU Emacs, the Swiss army knife of text editors 2024-08-01 7:49 ` martin rudalics via Bug reports for GNU Emacs, the Swiss army knife of text editors 2024-08-01 16:01 ` Juri Linkov 2024-10-08 17:53 ` Juri Linkov 2024-10-09 8:46 ` martin rudalics via Bug reports for GNU Emacs, the Swiss army knife of text editors 2024-10-09 16:17 ` Juri Linkov 2024-10-10 14:54 ` martin rudalics via Bug reports for GNU Emacs, the Swiss army knife of text editors 2024-10-10 18:28 ` Juri Linkov 2024-10-11 7:50 ` martin rudalics via Bug reports for GNU Emacs, the Swiss army knife of text editors 2024-10-13 6:18 ` Juri Linkov
Code repositories for project(s) associated with this public inbox https://git.savannah.gnu.org/cgit/emacs.git This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).