GNU bug report logs - #59862
quit-restore per window buffer

Previous Next

To add a comment to this bug, you must first unarchive it, by sending
a message to control AT debbugs.gnu.org, with unarchive 59862 in the body.
You can then email your comments to 59862 AT debbugs.gnu.org in the normal way.

Toggle the display of automated, internal messages from the tracker.

Message #5 received at submit <at> debbugs.gnu.org (full text, mbox):

From: Juri Linkov <juri <at> linkov.net>
To: bug-gnu-emacs <at> gnu.org
Subject: quit-restore per window buffer
Date: Tue, 06 Dec 2022 19:32:05 +0200

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.

Message #8 received at 59862 <at> debbugs.gnu.org (full text, mbox):

From: Juri Linkov <juri <at> linkov.net>
To: 59862 <at> debbugs.gnu.org
Cc: martin rudalics <rudalics <at> gmx.at>
Subject: Re: bug#59862: quit-restore per window buffer
Date: Sun, 02 Jun 2024 09:45:02 +0300

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.

Message #11 received at 59862 <at> debbugs.gnu.org (full text, mbox):

From: martin rudalics <rudalics <at> gmx.at>
To: Juri Linkov <juri <at> linkov.net>, 59862 <at> debbugs.gnu.org
Subject: Re: bug#59862: quit-restore per window buffer
Date: Mon, 3 Jun 2024 11:34:33 +0200

[Message part 1 (text/plain, inline)]

> 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

[quit-restore.diff (text/x-patch, attachment)]

Message #14 received at 59862 <at> debbugs.gnu.org (full text, mbox):

From: martin rudalics <rudalics <at> gmx.at>
To: Juri Linkov <juri <at> linkov.net>, 59862 <at> debbugs.gnu.org
Subject: Re: bug#59862: quit-restore per window buffer
Date: Mon, 3 Jun 2024 11:53:39 +0200

[Message part 1 (text/plain, inline)]

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

[quit-restore.diff (text/x-patch, attachment)]

Message #17 received at 59862 <at> debbugs.gnu.org (full text, mbox):

From: martin rudalics <rudalics <at> gmx.at>
To: Juri Linkov <juri <at> linkov.net>, 59862 <at> debbugs.gnu.org
Subject: Re: bug#59862: quit-restore per window buffer
Date: Mon, 3 Jun 2024 18:09:52 +0200

[Message part 1 (text/plain, inline)]

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

[quit-restore.diff (text/x-patch, attachment)]

Message #20 received at 59862 <at> debbugs.gnu.org (full text, mbox):

From: Juri Linkov <juri <at> linkov.net>
To: martin rudalics <rudalics <at> gmx.at>
Cc: 59862 <at> debbugs.gnu.org
Subject: Re: bug#59862: quit-restore per window buffer
Date: Tue, 04 Jun 2024 09:53:14 +0300

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

Message #23 received at 59862 <at> debbugs.gnu.org (full text, mbox):

From: Juri Linkov <juri <at> linkov.net>
To: martin rudalics <rudalics <at> gmx.at>
Cc: 59862 <at> debbugs.gnu.org
Subject: Re: bug#59862: quit-restore per window buffer
Date: Wed, 05 Jun 2024 19:56:14 +0300

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

Message #26 received at 59862 <at> debbugs.gnu.org (full text, mbox):

From: Juri Linkov <juri <at> linkov.net>
To: martin rudalics <rudalics <at> gmx.at>
Cc: 59862 <at> debbugs.gnu.org
Subject: Re: bug#59862: quit-restore per window buffer
Date: Tue, 11 Jun 2024 09:52:07 +0300

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

Message #29 received at 59862 <at> debbugs.gnu.org (full text, mbox):

From: martin rudalics <rudalics <at> gmx.at>
To: Juri Linkov <juri <at> linkov.net>
Cc: 59862 <at> debbugs.gnu.org
Subject: Re: bug#59862: quit-restore per window buffer
Date: Wed, 12 Jun 2024 10:57:34 +0200

> 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

Message #32 received at 59862 <at> debbugs.gnu.org (full text, mbox):

From: Juri Linkov <juri <at> linkov.net>
To: martin rudalics <rudalics <at> gmx.at>
Cc: 59862 <at> debbugs.gnu.org
Subject: Re: bug#59862: quit-restore per window buffer
Date: Thu, 13 Jun 2024 09:47:09 +0300

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

Message #35 received at 59862 <at> debbugs.gnu.org (full text, mbox):

From: martin rudalics <rudalics <at> gmx.at>
To: Juri Linkov <juri <at> linkov.net>
Cc: 59862 <at> debbugs.gnu.org
Subject: Re: bug#59862: quit-restore per window buffer
Date: Thu, 13 Jun 2024 10:21:32 +0200

> 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

Message #38 received at 59862 <at> debbugs.gnu.org (full text, mbox):

From: Juri Linkov <juri <at> linkov.net>
To: martin rudalics <rudalics <at> gmx.at>
Cc: 59862 <at> debbugs.gnu.org
Subject: Re: bug#59862: quit-restore per window buffer
Date: Fri, 14 Jun 2024 20:35:30 +0300

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

Message #41 received at 59862 <at> debbugs.gnu.org (full text, mbox):

From: martin rudalics <rudalics <at> gmx.at>
To: Juri Linkov <juri <at> linkov.net>
Cc: 59862 <at> debbugs.gnu.org
Subject: Re: bug#59862: quit-restore per window buffer
Date: Sat, 15 Jun 2024 10:41:54 +0200

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

Message #44 received at 59862 <at> debbugs.gnu.org (full text, mbox):

From: Juri Linkov <juri <at> linkov.net>
To: martin rudalics <rudalics <at> gmx.at>
Cc: 59862 <at> debbugs.gnu.org
Subject: Re: bug#59862: quit-restore per window buffer
Date: Sun, 16 Jun 2024 19:50:29 +0300

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

Message #47 received at 59862 <at> debbugs.gnu.org (full text, mbox):

From: martin rudalics <rudalics <at> gmx.at>
To: Juri Linkov <juri <at> linkov.net>
Cc: 59862 <at> debbugs.gnu.org
Subject: Re: bug#59862: quit-restore per window buffer
Date: Mon, 17 Jun 2024 16:48:16 +0200

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

Message #50 received at 59862 <at> debbugs.gnu.org (full text, mbox):

From: martin rudalics <rudalics <at> gmx.at>
To: Juri Linkov <juri <at> linkov.net>
Cc: 59862 <at> debbugs.gnu.org
Subject: Re: bug#59862: quit-restore per window buffer
Date: Mon, 8 Jul 2024 18:49:52 +0200

[Message part 1 (text/plain, inline)]

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)

[windows-and-buffers.diff (text/x-patch, attachment)]

Message #53 received at 59862 <at> debbugs.gnu.org (full text, mbox):

From: Juri Linkov <juri <at> linkov.net>
To: martin rudalics <rudalics <at> gmx.at>
Cc: 59862 <at> debbugs.gnu.org
Subject: Re: bug#59862: quit-restore per window buffer
Date: Tue, 09 Jul 2024 09:58:49 +0300

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

Message #56 received at 59862 <at> debbugs.gnu.org (full text, mbox):

From: martin rudalics <rudalics <at> gmx.at>
To: Juri Linkov <juri <at> linkov.net>
Cc: 59862 <at> debbugs.gnu.org
Subject: Re: bug#59862: quit-restore per window buffer
Date: Tue, 9 Jul 2024 10:52:03 +0200

> 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

Message #59 received at 59862 <at> debbugs.gnu.org (full text, mbox):

From: Juri Linkov <juri <at> linkov.net>
To: martin rudalics <rudalics <at> gmx.at>
Cc: 59862 <at> debbugs.gnu.org
Subject: Re: bug#59862: quit-restore per window buffer
Date: Wed, 10 Jul 2024 09:50:32 +0300

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

Message #62 received at 59862 <at> debbugs.gnu.org (full text, mbox):

From: martin rudalics <rudalics <at> gmx.at>
To: Juri Linkov <juri <at> linkov.net>
Cc: 59862 <at> debbugs.gnu.org
Subject: Re: bug#59862: quit-restore per window buffer
Date: Wed, 10 Jul 2024 11:16:24 +0200

> 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

Message #65 received at 59862 <at> debbugs.gnu.org (full text, mbox):

From: Juri Linkov <juri <at> linkov.net>
To: martin rudalics <rudalics <at> gmx.at>
Cc: 59862 <at> debbugs.gnu.org
Subject: Re: bug#59862: quit-restore per window buffer
Date: Thu, 11 Jul 2024 09:47:27 +0300

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

Message #68 received at 59862 <at> debbugs.gnu.org (full text, mbox):

From: martin rudalics <rudalics <at> gmx.at>
To: Juri Linkov <juri <at> linkov.net>
Cc: 59862 <at> debbugs.gnu.org
Subject: Re: bug#59862: quit-restore per window buffer
Date: Thu, 11 Jul 2024 10:36:46 +0200

> 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

Message #71 received at 59862 <at> debbugs.gnu.org (full text, mbox):

From: Juri Linkov <juri <at> linkov.net>
To: martin rudalics <rudalics <at> gmx.at>
Cc: 59862 <at> debbugs.gnu.org
Subject: Re: bug#59862: quit-restore per window buffer
Date: Fri, 12 Jul 2024 09:54:48 +0300

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

Message #74 received at 59862 <at> debbugs.gnu.org (full text, mbox):

From: martin rudalics <rudalics <at> gmx.at>
To: Juri Linkov <juri <at> linkov.net>
Cc: 59862 <at> debbugs.gnu.org
Subject: Re: bug#59862: quit-restore per window buffer
Date: Fri, 12 Jul 2024 10:20:32 +0200

> 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

Message #77 received at 59862 <at> debbugs.gnu.org (full text, mbox):

From: Juri Linkov <juri <at> linkov.net>
To: martin rudalics <rudalics <at> gmx.at>
Cc: 59862 <at> debbugs.gnu.org
Subject: Re: bug#59862: quit-restore per window buffer
Date: Sun, 14 Jul 2024 10:49:35 +0300

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

Message #80 received at 59862 <at> debbugs.gnu.org (full text, mbox):

From: martin rudalics <rudalics <at> gmx.at>
To: Juri Linkov <juri <at> linkov.net>
Cc: 59862 <at> debbugs.gnu.org
Subject: Re: bug#59862: quit-restore per window buffer
Date: Mon, 15 Jul 2024 09:32:33 +0200

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

Message #83 received at submit <at> debbugs.gnu.org (full text, mbox):

From: Björn Bidar <bjorn.bidar <at> thaodan.de>
To: martin rudalics via "Bug reports for GNU Emacs, the Swiss army knife of
 text editors" <bug-gnu-emacs <at> gnu.org>
Cc: martin rudalics <rudalics <at> gmx.at>, 59862 <at> debbugs.gnu.org,
 Juri Linkov <juri <at> linkov.net>
Subject: Re: bug#59862: quit-restore per window buffer
Date: Tue, 16 Jul 2024 09:09:25 +0300

martin rudalics via "Bug reports for GNU Emacs, the Swiss army knife of
text editors" <bug-gnu-emacs <at> 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.

Message #89 received at submit <at> debbugs.gnu.org (full text, mbox):

From: martin rudalics <rudalics <at> gmx.at>
To: Björn Bidar <bjorn.bidar <at> thaodan.de>,
 martin rudalics via Bug reports for GNU "Emacs, " the Swiss army knife of text
 editors <bug-gnu-emacs <at> gnu.org>
Cc: 59862 <at> debbugs.gnu.org, Juri Linkov <juri <at> linkov.net>
Subject: Re: bug#59862: quit-restore per window buffer
Date: Tue, 16 Jul 2024 10:22:34 +0200

> 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

Message #95 received at submit <at> debbugs.gnu.org (full text, mbox):

From: Björn Bidar <bjorn.bidar <at> thaodan.de>
To: martin rudalics via "Bug reports for GNU Emacs, the Swiss army knife of
 text editors" <bug-gnu-emacs <at> gnu.org>
Cc: martin rudalics <rudalics <at> gmx.at>, 59862 <at> debbugs.gnu.org, juri <at> linkov.net
Subject: Re: bug#59862: quit-restore per window buffer
Date: Wed, 17 Jul 2024 01:14:09 +0300

martin rudalics via "Bug reports for GNU Emacs, the Swiss army knife of
text editors" <bug-gnu-emacs <at> 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.

Message #101 received at 59862 <at> debbugs.gnu.org (full text, mbox):

From: martin rudalics <rudalics <at> gmx.at>
To: Björn Bidar <bjorn.bidar <at> thaodan.de>,
 martin rudalics via Bug reports for GNU "Emacs, " the Swiss army knife of text
 editors <bug-gnu-emacs <at> gnu.org>
Cc: 59862 <at> debbugs.gnu.org, juri <at> linkov.net
Subject: Re: bug#59862: quit-restore per window buffer
Date: Wed, 17 Jul 2024 11:23:31 +0200

[Message part 1 (text/plain, inline)]

> 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

[windows-and-buffers.diff (text/x-patch, attachment)]

Message #107 received at 59862 <at> debbugs.gnu.org (full text, mbox):

From: martin rudalics <rudalics <at> gmx.at>
To: Juri Linkov <juri <at> linkov.net>
Cc: 59862 <at> debbugs.gnu.org
Subject: Re: bug#59862: quit-restore per window buffer
Date: Tue, 30 Jul 2024 10:20:37 +0200

[Message part 1 (text/plain, inline)]

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

[window-buffer-changes.diff (text/x-patch, attachment)]

Message #110 received at 59862 <at> debbugs.gnu.org (full text, mbox):

From: Juri Linkov <juri <at> linkov.net>
To: martin rudalics <rudalics <at> gmx.at>
Cc: 59862 <at> debbugs.gnu.org
Subject: Re: bug#59862: quit-restore per window buffer
Date: Wed, 31 Jul 2024 20:30:36 +0300

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

Message #113 received at 59862 <at> debbugs.gnu.org (full text, mbox):

From: Juri Linkov <juri <at> linkov.net>
To: martin rudalics <rudalics <at> gmx.at>
Cc: 59862 <at> debbugs.gnu.org
Subject: Re: bug#59862: quit-restore per window buffer
Date: Thu, 01 Aug 2024 09:37:06 +0300

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

Message #116 received at 59862 <at> debbugs.gnu.org (full text, mbox):

From: martin rudalics <rudalics <at> gmx.at>
To: Juri Linkov <juri <at> linkov.net>
Cc: 59862 <at> debbugs.gnu.org
Subject: Re: bug#59862: quit-restore per window buffer
Date: Thu, 1 Aug 2024 09:49:52 +0200

> 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

Message #119 received at 59862 <at> debbugs.gnu.org (full text, mbox):

From: martin rudalics <rudalics <at> gmx.at>
To: Juri Linkov <juri <at> linkov.net>
Cc: 59862 <at> debbugs.gnu.org
Subject: Re: bug#59862: quit-restore per window buffer
Date: Thu, 1 Aug 2024 09:50:04 +0200

> 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

Message #122 received at 59862 <at> debbugs.gnu.org (full text, mbox):

From: Juri Linkov <juri <at> linkov.net>
To: martin rudalics <rudalics <at> gmx.at>
Cc: 59862 <at> debbugs.gnu.org
Subject: Re: bug#59862: quit-restore per window buffer
Date: Thu, 01 Aug 2024 19:01:59 +0300

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

Message #125 received at 59862 <at> debbugs.gnu.org (full text, mbox):

From: Juri Linkov <juri <at> linkov.net>
To: martin rudalics <rudalics <at> gmx.at>
Cc: 59862 <at> debbugs.gnu.org
Subject: Re: bug#59862: quit-restore per window buffer
Date: Tue, 08 Oct 2024 20:53:53 +0300

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

Message #128 received at 59862 <at> debbugs.gnu.org (full text, mbox):

From: martin rudalics <rudalics <at> gmx.at>
To: Juri Linkov <juri <at> linkov.net>
Cc: 59862 <at> debbugs.gnu.org
Subject: Re: bug#59862: quit-restore per window buffer
Date: Wed, 9 Oct 2024 10:46:05 +0200

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

Message #131 received at 59862 <at> debbugs.gnu.org (full text, mbox):

From: Juri Linkov <juri <at> linkov.net>
To: martin rudalics <rudalics <at> gmx.at>
Cc: 59862 <at> debbugs.gnu.org
Subject: Re: bug#59862: quit-restore per window buffer
Date: Wed, 09 Oct 2024 19:17:11 +0300

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

Message #134 received at 59862 <at> debbugs.gnu.org (full text, mbox):

From: martin rudalics <rudalics <at> gmx.at>
To: Juri Linkov <juri <at> linkov.net>
Cc: 59862 <at> debbugs.gnu.org
Subject: Re: bug#59862: quit-restore per window buffer
Date: Thu, 10 Oct 2024 16:54:02 +0200

> 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

Message #137 received at 59862 <at> debbugs.gnu.org (full text, mbox):

From: Juri Linkov <juri <at> linkov.net>
To: martin rudalics <rudalics <at> gmx.at>
Cc: 59862 <at> debbugs.gnu.org
Subject: Re: bug#59862: quit-restore per window buffer
Date: Thu, 10 Oct 2024 21:28:22 +0300

[Message part 1 (text/plain, inline)]

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

[quit-restore-tab.patch (text/x-diff, inline)]

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

Message #140 received at 59862 <at> debbugs.gnu.org (full text, mbox):

From: martin rudalics <rudalics <at> gmx.at>
To: Juri Linkov <juri <at> linkov.net>
Cc: 59862 <at> debbugs.gnu.org
Subject: Re: bug#59862: quit-restore per window buffer
Date: Fri, 11 Oct 2024 09:50:44 +0200

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

Message #143 received at 59862 <at> debbugs.gnu.org (full text, mbox):

From: Juri Linkov <juri <at> linkov.net>
To: martin rudalics <rudalics <at> gmx.at>
Cc: 59862 <at> debbugs.gnu.org
Subject: Re: bug#59862: quit-restore per window buffer
Date: Sun, 13 Oct 2024 09:18:00 +0300

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

Message #146 received at 59862 <at> debbugs.gnu.org (full text, mbox):

From: Juri Linkov <juri <at> linkov.net>
To: martin rudalics <rudalics <at> gmx.at>
Cc: Ship Mints <shipmints <at> gmail.com>, 59862 <at> debbugs.gnu.org
Subject: Re: bug#59862: quit-restore per window buffer
Date: Thu, 03 Apr 2025 19:29:01 +0300

close 59862 31.0.50
thanks

>> +     ((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, now everything fixed and closed.

Previous Next

GNU bug tracking system
Copyright (C) 1999 Darren O. Benham, 1997,2003 nCipher Corporation Ltd, 1994-97 Ian Jackson.