GNU bug report logs - #46904
Non-unique windows produced by window-state-put

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 46904 in the body.
You can then email your comments to 46904 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: Non-unique windows produced by window-state-put
Date: Wed, 03 Mar 2021 22:13:24 +0200

We have a problem in window-state-put.  For example, in emacs -Q
evaluate this several times:

(progn
  (window-state-put
   (window-state-get
    (frame-root-window (selected-frame))
    'writable)
   (frame-root-window (selected-frame)) 'safe)
  (selected-window))

Every time the selected window remains the same:

  #<window 3 on *scratch*>

But after splitting the window with e.g. 'C-x 2',
evaluating the same every time creates a new window
that it's expected to do even when there is only one window
on the frame.

I don't know how severe are the consequences.  The only side-effect I noticed
that prev-buffers of restored windows are the same on different tabs because
they share the same window.

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

From: martin rudalics <rudalics <at> gmx.at>
To: Juri Linkov <juri <at> linkov.net>, 46904 <at> debbugs.gnu.org
Subject: Re: bug#46904: Non-unique windows produced by window-state-put
Date: Thu, 4 Mar 2021 09:35:26 +0100

> We have a problem in window-state-put.  For example, in emacs -Q
> evaluate this several times:
>
> (progn
>    (window-state-put
>     (window-state-get
>      (frame-root-window (selected-frame))
>      'writable)
>     (frame-root-window (selected-frame)) 'safe)
>    (selected-window))
>
> Every time the selected window remains the same:
>
>    #<window 3 on *scratch*>
>
> But after splitting the window with e.g. 'C-x 2',
> evaluating the same every time creates a new window
> that it's expected to do even when there is only one window
> on the frame.

You mean we should do that

            ;; Create a new window to replace the existing one.
            (setq window (prog1 (split-window window)
                           (delete-window window)))))

in the one window case too?  This would be certainly more consistent but
I do not remember any more why this is needed in the multiple windows
case.

> I don't know how severe are the consequences.  The only side-effect I noticed
> that prev-buffers of restored windows are the same on different tabs because
> they share the same window.

If that is a problem and the above would help, let's do it.  It couldn't
possibly harm unless the root window is too small to get split and we
want to put only one window there; so we should handle that special case.

martin

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

From: Juri Linkov <juri <at> linkov.net>
To: martin rudalics <rudalics <at> gmx.at>
Cc: 46904 <at> debbugs.gnu.org
Subject: Re: bug#46904: Non-unique windows produced by window-state-put
Date: Thu, 04 Mar 2021 11:39:09 +0200

>>
>> Every time the selected window remains the same:
>>
>>    #<window 3 on *scratch*>
>>
>> But after splitting the window with e.g. 'C-x 2',
>> evaluating the same every time creates a new window
>> that it's expected to do even when there is only one window
>> on the frame.
>
> You mean we should do that
>
>             ;; Create a new window to replace the existing one.
>             (setq window (prog1 (split-window window)
>                            (delete-window window)))))
>
> in the one window case too?  This would be certainly more consistent but

This looks like the right fix.

> I do not remember any more why this is needed in the multiple windows
> case.

And I do not remember it too, maybe the current logic handled some use cases,
maybe not.

>> I don't know how severe are the consequences.  The only side-effect I noticed
>> that prev-buffers of restored windows are the same on different tabs because
>> they share the same window.
>
> If that is a problem and the above would help, let's do it.  It couldn't
> possibly harm unless the root window is too small to get split and we
> want to put only one window there; so we should handle that special case.

I'll try this, thanks.

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

From: martin rudalics <rudalics <at> gmx.at>
To: Juri Linkov <juri <at> linkov.net>
Cc: 46904 <at> debbugs.gnu.org
Subject: Re: bug#46904: Non-unique windows produced by window-state-put
Date: Thu, 4 Mar 2021 11:34:58 +0100

>> It couldn't
>> possibly harm unless the root window is too small to get split and we
>> want to put only one window there; so we should handle that special case.
>
> I'll try this, thanks.

I'd first try a vertical, then a horizontal split.  Give up if both
fail.

martin

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

From: Juri Linkov <juri <at> linkov.net>
To: martin rudalics <rudalics <at> gmx.at>
Cc: 46904 <at> debbugs.gnu.org
Subject: Re: bug#46904: Non-unique windows produced by window-state-put
Date: Thu, 04 Mar 2021 19:57:36 +0200

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

>>> It couldn't
>>> possibly harm unless the root window is too small to get split and we
>>> want to put only one window there; so we should handle that special case.
>>
>> I'll try this, thanks.
>
> I'd first try a vertical, then a horizontal split.  Give up if both
> fail.

Maybe something like this:

[window-state-put.patch (text/x-diff, inline)]

diff --git a/lisp/window.el b/lisp/window.el
index cfd9876ed0..d750e16626 100644
--- a/lisp/window.el
+++ b/lisp/window.el
@@ -6229,10 +6229,15 @@ window-state-put
                               (throw 'live window)))
                           root))
                      (selected-window)))
-      (delete-other-windows-internal window root)
-      ;; Create a new window to replace the existing one.
-      (setq window (prog1 (split-window window)
-                     (delete-window window)))))
+      (delete-other-windows-internal window root)))
+
+  ;; Create a new window to replace the existing one.
+  (let ((new-window
+         (or (ignore-errors (split-window (selected-window)))
+             (ignore-errors (split-window (selected-window) nil t)))))
+    (when new-window
+      (delete-window window)
+      (setq window new-window)))
 
   (set-window-dedicated-p window nil)
 

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

From: Juri Linkov <juri <at> linkov.net>
To: martin rudalics <rudalics <at> gmx.at>
Cc: 46904 <at> debbugs.gnu.org
Subject: Re: bug#46904: Non-unique windows produced by window-state-put
Date: Thu, 04 Mar 2021 22:31:34 +0200

>> I'd first try a vertical, then a horizontal split.  Give up if both
>> fail.
>
> Maybe something like this:
>
> @@ -6229,10 +6229,15 @@ window-state-put
> +  ;; Create a new window to replace the existing one.
> +  (let ((new-window
> +         (or (ignore-errors (split-window (selected-window)))
> +             (ignore-errors (split-window (selected-window) nil t)))))
> +    (when new-window
> +      (delete-window window)
> +      (setq window new-window)))

Actually with this patch window-swap-states fails with the error
"Wrong type argument: stringp, nil" in:

      ;; Swap basic states.
      (window-state-put state-1 window-2 t)
      (window-state-put state-2 window-1 t)
      ;; Swap overlays with `window' property.
      (with-current-buffer (window-buffer window-1)

Here '(window-buffer window-1)' returns nil because
window-1 doesn't exist anymore after calling window-state-put.

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

From: martin rudalics <rudalics <at> gmx.at>
To: Juri Linkov <juri <at> linkov.net>
Cc: 46904 <at> debbugs.gnu.org
Subject: Re: bug#46904: Non-unique windows produced by window-state-put
Date: Fri, 5 Mar 2021 10:10:31 +0100

> Actually with this patch window-swap-states fails with the error
> "Wrong type argument: stringp, nil" in:
>
>        ;; Swap basic states.
>        (window-state-put state-1 window-2 t)
>        (window-state-put state-2 window-1 t)
>        ;; Swap overlays with `window' property.
>        (with-current-buffer (window-buffer window-1)
>
> Here '(window-buffer window-1)' returns nil because
> window-1 doesn't exist anymore after calling window-state-put.

We could easily handle that by having `window-state-put' return the
window actually used for putting the state into and use that in
`window-swap-states'.  Yet someone else might use `window-state-put' in
much the same way as `window-swap-states' does and we have a quite
incompatible change: We nowhere say that the window used as second
argument of `window-state-put' will be live if it was so before, but we
neither say that it can be deleted.  So I'm not sure what to do ...

martin

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

From: Juri Linkov <juri <at> linkov.net>
To: martin rudalics <rudalics <at> gmx.at>
Cc: 46904 <at> debbugs.gnu.org
Subject: Re: bug#46904: Non-unique windows produced by window-state-put
Date: Sun, 07 Mar 2021 20:43:40 +0200

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

>> Actually with this patch window-swap-states fails with the error
>> "Wrong type argument: stringp, nil" in:
>>
>>        ;; Swap basic states.
>>        (window-state-put state-1 window-2 t)
>>        (window-state-put state-2 window-1 t)
>>        ;; Swap overlays with `window' property.
>>        (with-current-buffer (window-buffer window-1)
>>
>> Here '(window-buffer window-1)' returns nil because
>> window-1 doesn't exist anymore after calling window-state-put.
>
> We could easily handle that by having `window-state-put' return the
> window actually used for putting the state into and use that in
> `window-swap-states'.  Yet someone else might use `window-state-put' in
> much the same way as `window-swap-states' does and we have a quite
> incompatible change: We nowhere say that the window used as second
> argument of `window-state-put' will be live if it was so before, but we
> neither say that it can be deleted.  So I'm not sure what to do ...

Indeed, this would be an incompatible change.

So better to fix callers not to use frame-root-window
that is unreliable: sometimes frame-root-window returns
an internal window, sometimes a live window:

[window-state-put-without-frame-root-window.patch (text/x-diff, inline)]

diff --git a/lisp/tab-bar.el b/lisp/tab-bar.el
index 917b5e496b..a7d905184b 100644
--- a/lisp/tab-bar.el
+++ b/lisp/tab-bar.el
@@ -766,7 +772,7 @@ tab-bar-select-tab
                      tab-bar-history-forward)))
 
          (ws
-          (window-state-put ws (frame-root-window (selected-frame)) 'safe)))
+          (window-state-put ws nil 'safe)))
 
         (setq tab-bar-history-omit t)
 

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

From: martin rudalics <rudalics <at> gmx.at>
To: Juri Linkov <juri <at> linkov.net>
Cc: 46904 <at> debbugs.gnu.org
Subject: Re: bug#46904: Non-unique windows produced by window-state-put
Date: Mon, 8 Mar 2021 09:26:21 +0100

> So better to fix callers not to use frame-root-window
> that is unreliable: sometimes frame-root-window returns
> an internal window, sometimes a live window:

To emulate `set-window-configuration' via `window-state-put' you have
to put the state into the root window.

martin

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

From: Juri Linkov <juri <at> linkov.net>
To: martin rudalics <rudalics <at> gmx.at>
Cc: 46904 <at> debbugs.gnu.org
Subject: Re: bug#46904: Non-unique windows produced by window-state-put
Date: Mon, 08 Mar 2021 19:18:16 +0200

>> So better to fix callers not to use frame-root-window
>> that is unreliable: sometimes frame-root-window returns
>> an internal window, sometimes a live window:
>
> To emulate `set-window-configuration' via `window-state-put' you have
> to put the state into the root window.

When 'frame-root-window' returns an internal window,
then we don't put the state into the root window anyway,
we create a new window.  The patch does the same for the
case when there is only one window too, so both cases
will provide the same consistent result.

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

From: martin rudalics <rudalics <at> gmx.at>
To: Juri Linkov <juri <at> linkov.net>
Cc: 46904 <at> debbugs.gnu.org
Subject: Re: bug#46904: Non-unique windows produced by window-state-put
Date: Mon, 8 Mar 2021 19:06:53 +0100

> When 'frame-root-window' returns an internal window,
> then we don't put the state into the root window anyway,
> we create a new window.  The patch does the same for the
> case when there is only one window too, so both cases
> will provide the same consistent result.

Yes.  But how would you handle `window-swap-states' now?

martin

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

From: Juri Linkov <juri <at> linkov.net>
To: martin rudalics <rudalics <at> gmx.at>
Cc: 46904 <at> debbugs.gnu.org
Subject: Re: bug#46904: Non-unique windows produced by window-state-put
Date: Mon, 08 Mar 2021 20:24:36 +0200

>> When 'frame-root-window' returns an internal window,
>> then we don't put the state into the root window anyway,
>> we create a new window.  The patch does the same for the
>> case when there is only one window too, so both cases
>> will provide the same consistent result.
>
> Yes.  But how would you handle `window-swap-states' now?

No changes are planned in window.el, only in tab-bar.el.

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

From: Juri Linkov <juri <at> linkov.net>
To: martin rudalics <rudalics <at> gmx.at>
Cc: 46904 <at> debbugs.gnu.org
Subject: Re: bug#46904: Non-unique windows produced by window-state-put
Date: Tue, 09 Mar 2021 19:28:09 +0200

tags 46904 fixed
close 46904 28.0.50
thanks

>>> When 'frame-root-window' returns an internal window,
>>> then we don't put the state into the root window anyway,
>>> we create a new window.  The patch does the same for the
>>> case when there is only one window too, so both cases
>>> will provide the same consistent result.
>>
>> Yes.  But how would you handle `window-swap-states' now?
>
> No changes are planned in window.el, only in tab-bar.el.

Now pushed to master in commit 29458ec7d2.

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

From: Juri Linkov <juri <at> linkov.net>
To: martin rudalics <rudalics <at> gmx.at>
Cc: 46904 <at> debbugs.gnu.org
Subject: Re: bug#46904: Non-unique windows produced by window-state-put
Date: Tue, 27 Apr 2021 20:19:21 +0300

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

>> I don't know how severe are the consequences.  The only side-effect I noticed
>> that prev-buffers of restored windows are the same on different tabs because
>> they share the same window.
>
> If that is a problem and the above would help, let's do it.  It couldn't
> possibly harm unless the root window is too small to get split and we
> want to put only one window there; so we should handle that special case.

The original problem was that prev-buffers of restored windows are the
same on different tabs.  This was fixed by always creating a new window.
However, the same problem still exists, but for a different reason.

Most windows have current buffer duplicated in prev-buffers.  So when
set-window-prev-buffers in window--state-put-2 sets prev-buffers to a list
with only current buffer, this means as if there are no prev-buffers.

But when a window has an empty list of prev-buffers, then the window
parameter of prev-buffers is not cleared, and still contains a random
buffer from some previous state.  This patch fixes it:

[window--state-put-2-prev-buffers.patch (text/x-diff, inline)]

diff --git a/lisp/window.el b/lisp/window.el
index 036eb271ee..fc4ca303b0 100644
--- a/lisp/window.el
+++ b/lisp/window.el
@@ -6140,7 +6140,8 @@ window--state-put-2
                                        (setq buffer (get-buffer buffer))
                                        (when (buffer-live-p buffer) buffer))
                                      next-buffers))))
-                (when prev-buffers
+                (if (not prev-buffers)
+                    (set-window-prev-buffers window nil)
                   (set-window-prev-buffers
                    window
                    (delq nil (mapcar (lambda (entry)

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

From: Juri Linkov <juri <at> linkov.net>
To: martin rudalics <rudalics <at> gmx.at>
Cc: 46904 <at> debbugs.gnu.org
Subject: Re: bug#46904: Non-unique windows produced by window-state-put
Date: Wed, 28 Apr 2021 23:25:43 +0300

> But when a window has an empty list of prev-buffers, then the window
> parameter of prev-buffers is not cleared, and still contains a random
> buffer from some previous state.  This patch fixes it:

Now pushed to master in commit 0e8c862885.

Previous Next

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