bug#32850: 27.0.50; window-swap-states doesn't swap window prev/next-buffers

bug#32850: 27.0.50; window-swap-states doesn't swap window prev/next-buffers

[Juri Linkov]

For a long time I've been using my own implementation that swaps window states:

  (defun rotate-window-buffers (&optional n)
    "Exchange buffers in all windows N times.
  With positive N, it uses the window order of `window-list'.
  With negative N, does this in the reverse order."
    (interactive "p")
    (let* ((ws (window-list))
           (ls (mapcar (lambda (w)
                         (list (window-buffer w)
                               (window-point w)
                               (window-next-buffers w)
                               (window-prev-buffers w)))
                       ws))
           (n  (mod (or n 1) (length ws))))
      (dolist (w (append (last ws n) (butlast ws n)))
        (let ((l (pop ls)))
          (set-window-buffer w (nth 0 l))
          (set-window-point  w (nth 1 l))
          (set-window-next-buffers w (nth 2 l))
          (set-window-prev-buffers w (nth 3 l))))))

Now I wanted to throw it away and to use window-swap-states
from Emacs core that should do the same.  But after trying
I see that window-swap-states breaks the order of window buffers
for 'C-x <C-left>' (previous-buffer) and 'C-x <C-right>' (next-buffer)
because it doesn't swap window-prev-buffers and window-next-buffers.

Shouldn't window-swap-states also swap prev/next buffers?

bug#32850: 27.0.50; window-swap-states doesn't swap window prev/next-buffers

[martin rudalics]

 > I see that window-swap-states breaks the order of window buffers
 > for 'C-x <C-left>' (previous-buffer) and 'C-x <C-right>' (next-buffer)
 > because it doesn't swap window-prev-buffers and window-next-buffers.
 >
 > Shouldn't window-swap-states also swap prev/next buffers?

So you want to change the course of history.  Could you provide some
rationale for such a change?

martin

bug#32850: 27.0.50; window-swap-states doesn't swap window prev/next-buffers

[Juri Linkov]

>> I see that window-swap-states breaks the order of window buffers
>> for 'C-x <C-left>' (previous-buffer) and 'C-x <C-right>' (next-buffer)
>> because it doesn't swap window-prev-buffers and window-next-buffers.
>>
>> Shouldn't window-swap-states also swap prev/next buffers?
>
> So you want to change the course of history.  Could you provide some
> rationale for such a change?

Actually not to change the course of history, but keep it together with
its window, or rather an imaginary window - when using window-swap-states,
it looks like windows are swapped: since window's buffer is moved
to a new location, the feeling is that the whole window is moved to
a new place, because what defines a window is its state more than
a location on the screen, so to not break the history of buffers
in the window, prev/next buffers should be swapped as well, to
keep the chain of buffers: prev-buffers - current buffer - next-buffer,
so 'C-x <C-left>' (previous-buffer) and 'C-x <C-right>' (next-buffer)
will navigate in the preserved history in the same order it navigated
in the old window location.

bug#32850: 27.0.50; window-swap-states doesn't swap window prev/next-buffers

[martin rudalics]

 > Actually not to change the course of history, but keep it together with
 > its window, or rather an imaginary window - when using window-swap-states,
 > it looks like windows are swapped: since window's buffer is moved
 > to a new location, the feeling is that the whole window is moved to
 > a new place, because what defines a window is its state more than
 > a location on the screen, so to not break the history of buffers
 > in the window, prev/next buffers should be swapped as well, to
 > keep the chain of buffers: prev-buffers - current buffer - next-buffer,
 > so 'C-x <C-left>' (previous-buffer) and 'C-x <C-right>' (next-buffer)
 > will navigate in the preserved history in the same order it navigated
 > in the old window location.

Swapping the states of two windows does not swap their identities.
That is, the windows themselves remain where they are, only their
buffers change.  If we changed their previous and next buffers, we
would tell a lie.  And we might introduce some slight inconsistency
because the previous/next buffers of a window are no more elements of
the 'buffer-list' of their respective frames when the windows are on
different frames.  I doubt that has any consequences, though ...

So if you want this change, please provide an option say
'window-swap-states-swap-history' and, if that is non-nil, support the
changes from your 'rotate-window-buffers'.

Thanks, martin

bug#32850: 27.0.50; window-swap-states doesn't swap window prev/next-buffers

[Juri Linkov]

>> Actually not to change the course of history, but keep it together with
>> its window, or rather an imaginary window - when using window-swap-states,
>> it looks like windows are swapped: since window's buffer is moved
>> to a new location, the feeling is that the whole window is moved to
>> a new place, because what defines a window is its state more than
>> a location on the screen, so to not break the history of buffers
>> in the window, prev/next buffers should be swapped as well, to
>> keep the chain of buffers: prev-buffers - current buffer - next-buffer,
>> so 'C-x <C-left>' (previous-buffer) and 'C-x <C-right>' (next-buffer)
>> will navigate in the preserved history in the same order it navigated
>> in the old window location.
>
> Swapping the states of two windows does not swap their identities.
> That is, the windows themselves remain where they are, only their
> buffers change.  If we changed their previous and next buffers, we
> would tell a lie.  And we might introduce some slight inconsistency
> because the previous/next buffers of a window are no more elements of
> the 'buffer-list' of their respective frames when the windows are on
> different frames.  I doubt that has any consequences, though ...

If a window buffer is part of the window state and not window identity,
then prev/next-buffers should be swapped together with the window buffer
as part of the window state, and not window identity.

IOW, the window buffer is inseparable from the list of prev/next-buffers.
For example, in such sequences of buffers:

  *info*<1> *info*<2> *info*<3> *info*<4> *info*<5>
  prev buffers      current buffer     next buffers

and

  *grep*<1> *grep*<2> *grep*<3> *grep*<4> *grep*<5>
  prev buffers      current buffer     next buffers

window-swap-states currently breaks the list of related buffers,
e.g. after swapping only the window buffers, but not window
prev/next-buffers:

  *info*<1> *info*<2> *grep*<3> *info*<4> *info*<5>
  prev buffers      current buffer     next buffers

and

  *grep*<1> *grep*<2> *info*<3> *grep*<4> *grep*<5>
  prev buffers      current buffer     next buffers

'C-x <C-left>' and 'C-x <C-right>' become broken.

> So if you want this change, please provide an option say
> 'window-swap-states-swap-history' and, if that is non-nil, support the
> changes from your 'rotate-window-buffers'.

Forget 'rotate-window-buffers', it's a bad name and I want to throw it away.
What I need is for Emacs to provide a consistent behavior by default.

bug#32850: 27.0.50; window-swap-states doesn't swap window prev/next-buffers

[martin rudalics]

 > For example, in such sequences of buffers:
 >
 >    *info*<1> *info*<2> *info*<3> *info*<4> *info*<5>
 >    prev buffers      current buffer     next buffers
 >
 > and
 >
 >    *grep*<1> *grep*<2> *grep*<3> *grep*<4> *grep*<5>
 >    prev buffers      current buffer     next buffers
 >
 > window-swap-states currently breaks the list of related buffers,
 > e.g. after swapping only the window buffers, but not window
 > prev/next-buffers:
 >
 >    *info*<1> *info*<2> *grep*<3> *info*<4> *info*<5>
 >    prev buffers      current buffer     next buffers
 >
 > and
 >
 >    *grep*<1> *grep*<2> *info*<3> *grep*<4> *grep*<5>
 >    prev buffers      current buffer     next buffers

This looks consistent to me.  The respectively "current" buffers have
been swapped and the previous and next buffers of each window remain
the same.

 > 'C-x <C-left>' and 'C-x <C-right>' become broken.

In which sense?

 > Forget 'rotate-window-buffers', it's a bad name and I want to throw it away.
 > What I need is for Emacs to provide a consistent behavior by default.

So far I'm missing you.  How would you define consistency in this
context?

martin

bug#32850: 27.0.50; window-swap-states doesn't swap window prev/next-buffers

[Juri Linkov]

> So far I'm missing you.  How would you define consistency in this
> context?

The root of the problem is not in window-swap-states, it's in
window-state-get that it it relies upon.  For a long time I have
been using a package that keeps a list of window-configurations
and switches between them in the same frame.  But the problem
is that we have no serialization for window-configurations, so
no way to save in the desktop file and restore in another session.

With the invention of window-state-get I tried to replace all calls
of current-window-configuration with window-state-get (and
set-window-configuration with window-state-put), and it worked
perfectly (saved and restored serialized window-configurations
in the desktop file), but still the problem that prevents its use
is that it doesn't store prev/next-buffers that is a very important
feature.

bug#32850: 27.0.50; window-swap-states doesn't swap window prev/next-buffers

[martin rudalics]

 > The root of the problem is not in window-swap-states, it's in
 > window-state-get that it it relies upon.  For a long time I have
 > been using a package that keeps a list of window-configurations
 > and switches between them in the same frame.  But the problem
 > is that we have no serialization for window-configurations, so
 > no way to save in the desktop file and restore in another session.
 >
 > With the invention of window-state-get I tried to replace all calls
 > of current-window-configuration with window-state-get (and
 > set-window-configuration with window-state-put), and it worked
 > perfectly (saved and restored serialized window-configurations
 > in the desktop file), but still the problem that prevents its use
 > is that it doesn't store prev/next-buffers that is a very important
 > feature.

Please provide a patch that makes the behavior you want optional.

I could try to come up with a patch myself but since you already have
coded such a thing why duplicate the efforts?  IIUC we also have to
decide whether and how to expand your code to desktop saving and how
to handle buffers that got killed in between saving and restoring.

Later we can decide whether to install it as default.

Thanks, martin

bug#32850: 27.0.50; window-swap-states doesn't swap window prev/next-buffers

[Juri Linkov]

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

>> The root of the problem is not in window-swap-states, it's in
>> window-state-get that it it relies upon.  For a long time I have
>> been using a package that keeps a list of window-configurations
>> and switches between them in the same frame.  But the problem
>> is that we have no serialization for window-configurations, so
>> no way to save in the desktop file and restore in another session.
>>
>> With the invention of window-state-get I tried to replace all calls
>> of current-window-configuration with window-state-get (and
>> set-window-configuration with window-state-put), and it worked
>> perfectly (saved and restored serialized window-configurations
>> in the desktop file), but still the problem that prevents its use
>> is that it doesn't store prev/next-buffers that is a very important
>> feature.
>
> Please provide a patch that makes the behavior you want optional.

Are there cases where this behavior is undesirable?

> I could try to come up with a patch myself but since you already have
> coded such a thing why duplicate the efforts?  IIUC we also have to
> decide whether and how to expand your code to desktop saving and how
> to handle buffers that got killed in between saving and restoring.

This patch handles killed buffers, so it works for desktop saving as well
(because the desktop restores the frameset only after it reads all buffers):


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: window-state-prev-next-buffers.1.patch --]
[-- Type: text/x-diff, Size: 4134 bytes --]

diff --git a/lisp/window.el b/lisp/window.el
index 0a42dae6ca..a41d7a6f3c 100644
--- a/lisp/window.el
+++ b/lisp/window.el
@@ -5541,6 +5541,10 @@ window--state-get-1
 	   (t 'leaf)))
 	 (buffer (window-buffer window))
 	 (selected (eq window (selected-window)))
+	 (next-buffers (when (window-live-p window)
+			 (window-next-buffers window)))
+	 (prev-buffers (when (window-live-p window)
+			 (window-prev-buffers window)))
 	 (head
 	  `(,type
             ,@(unless (window-next-sibling window) `((last . t)))
@@ -5593,7 +5597,22 @@ window--state-get-1
 		     (start . ,(if writable
                                    start
                                  (with-current-buffer buffer
-                                   (copy-marker start))))))))))
+                                   (copy-marker start))))))))
+            ,@(when next-buffers
+                `((next-buffers . ,(mapcar (lambda (buffer)
+                                           (buffer-name buffer))
+                                         next-buffers))))
+            ,@(when prev-buffers
+                `((prev-buffers .
+                   ,(mapcar (lambda (entry)
+                              (list (buffer-name (nth 0 entry))
+                                    (if writable
+                                        (marker-position (nth 1 entry))
+                                      (nth 1 entry))
+                                    (if writable
+                                        (marker-position (nth 2 entry))
+                                      (nth 2 entry))))
+                            prev-buffers))))))
 	 (tail
 	  (when (memq type '(vc hc))
 	    (let (list)
@@ -5736,7 +5755,9 @@ window--state-put-2
     (let ((window (car item))
 	  (combination-limit (cdr (assq 'combination-limit item)))
 	  (parameters (cdr (assq 'parameters item)))
-	  (state (cdr (assq 'buffer item))))
+	  (state (cdr (assq 'buffer item)))
+	  (next-buffers (cdr (assq 'next-buffers item)))
+	  (prev-buffers (cdr (assq 'prev-buffers item))))
       (when combination-limit
 	(set-window-combination-limit window combination-limit))
       ;; Reset window's parameters and assign saved ones (we might want
@@ -5748,7 +5769,8 @@ window--state-put-2
 	  (set-window-parameter window (car parameter) (cdr parameter))))
       ;; Process buffer related state.
       (when state
-	(let ((buffer (get-buffer (car state))))
+	(let ((buffer (get-buffer (car state)))
+	      (state (cdr state)))
 	  (if buffer
 	      (with-current-buffer buffer
 		(set-window-buffer window buffer)
@@ -5817,7 +5839,26 @@ window--state-put-2
 		  (set-window-point window (cdr (assq 'point state))))
 		;; Select window if it's the selected one.
 		(when (cdr (assq 'selected state))
-		  (select-window window)))
+		  (select-window window))
+                (when next-buffers
+                  (set-window-next-buffers
+                   window
+                   (delq nil (mapcar (lambda (buffer)
+                                       (setq buffer (get-buffer buffer))
+                                       (when (buffer-live-p buffer) buffer))
+                                     next-buffers))))
+                (when prev-buffers
+                  (set-window-prev-buffers
+                   window
+                   (delq nil (mapcar (lambda (entry)
+                                       (let ((buffer (get-buffer (nth 0 entry))))
+                                         (when (buffer-live-p buffer)
+                                           (list buffer
+                                                 (set-marker (make-marker)
+                                                             (nth 1 entry) buffer)
+                                                 (set-marker (make-marker)
+                                                             (nth 2 entry) buffer)))))
+                                     prev-buffers)))))
 	    ;; We don't want to raise an error in case the buffer does
 	    ;; not exist anymore, so we switch to a previous one and
 	    ;; save the window with the intention of deleting it later

bug#32850: 27.0.50; window-swap-states doesn't swap window prev/next-buffers

[martin rudalics]

 >> Please provide a patch that makes the behavior you want optional.
 >
 > Are there cases where this behavior is undesirable?

If in doubt make it customizable.  With your bug reports I currently
have the "return space to which window after deleting it", "which
window to select after deleting the selected one", "which window
'display-buffer-pop-up-window' shall split" and "which window
'display-buffer-use-some-window' shall use" all as candidates for
customization.  Hence having 'window-swap-states-swap-buffer-lists'
too would probably not bother anyone

But if you are sure that it won't harm anyone to make it unconditional
leave it well alone.  We can always add a customization later if
someone needs it.  Personally, I don't care.

 > This patch handles killed buffers, so it works for desktop saving as well
 > (because the desktop restores the frameset only after it reads all buffers):

Please install so we can see whether there are any problems (in
particular with the desktop which I don't use).

Thanks, martin

bug#32850: 27.0.50; window-swap-states doesn't swap window prev/next-buffers

[Juri Linkov]

> But if you are sure that it won't harm anyone to make it unconditional
> leave it well alone.  We can always add a customization later if
> someone needs it.  Personally, I don't care.

I'd like to make it customizable by using the existing customization in
the arg WRITABLE of window-state-get and window-persistent-parameters,
even though formally prev/next-buffers is not a window parameter (maybe
it should be, I don't know).

>> This patch handles killed buffers, so it works for desktop saving as well
>> (because the desktop restores the frameset only after it reads all buffers):
>
> Please install so we can see whether there are any problems (in
> particular with the desktop which I don't use).

Done, with a small change: even though set-marker is idempotent in regard
to its POSITION arg (i.e. if POSITION is a marker, it creates an identical
marker), I added a check to not create a new one.  OTOH, get-buffer is
idempotent too, but it seems window-state-put never receives a structure
with buffer objects, and I'm not sure why window-state-get should always
use buffer-name regardless of the value WRITABLE, i.e. why should it return
buffer names as strings instead of buffer objects even when WRITABLE is nil?

bug#32850: 27.0.50; window-swap-states doesn't swap window prev/next-buffers

[martin rudalics]

 > I'd like to make it customizable by using the existing customization in
 > the arg WRITABLE of window-state-get and window-persistent-parameters,
 > even though formally prev/next-buffers is not a window parameter (maybe
 > it should be, I don't know).

Always keep in mind that prev_buffers and next_buffers need special
treatment in mark_object to make sure that dead buffers from window
configurations stored somewhere (for example in a register) get their
entries deleted and can be eventually reclaimed.  This is something I
completely disregarded when writing the original code for navigating
these buffer lists.  Stefan then wrote the code to do that in
mark_object.

 > Done, with a small change: even though set-marker is idempotent in regard
 > to its POSITION arg (i.e. if POSITION is a marker, it creates an identical
 > marker), I added a check to not create a new one.  OTOH, get-buffer is
 > idempotent too, but it seems window-state-put never receives a structure
 > with buffer objects, and I'm not sure why window-state-get should always
 > use buffer-name regardless of the value WRITABLE, i.e. why should it return
 > buffer names as strings instead of buffer objects even when WRITABLE is nil?

Maybe because I didn't care about non-writable states back then.  But
you're obviously right.  Buffers can be renamed at will and in that
case we might have a state with a non-existent buffer or even buffers
with names switched and therefore invalid values of start or point.

So yes: Please use a buffer object for non-writable states.

martin

bug#32850: 27.0.50; window-swap-states doesn't swap window prev/next-buffers

[Juri Linkov]

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

>> I'd like to make it customizable by using the existing customization in
>> the arg WRITABLE of window-state-get and window-persistent-parameters,
>> even though formally prev/next-buffers is not a window parameter (maybe
>> it should be, I don't know).
>
> Always keep in mind that prev_buffers and next_buffers need special
> treatment in mark_object to make sure that dead buffers from window
> configurations stored somewhere (for example in a register) get their
> entries deleted and can be eventually reclaimed.  This is something I
> completely disregarded when writing the original code for navigating
> these buffer lists.  Stefan then wrote the code to do that in
> mark_object.

When testing the previous patch, I noticed that killed buffers automatically
disappear from prev/next-buffers lists, so there are no #<killed buffer> remains.
Is it handled by code in kill-buffer?

>> Done, with a small change: even though set-marker is idempotent in regard
>> to its POSITION arg (i.e. if POSITION is a marker, it creates an identical
>> marker), I added a check to not create a new one.  OTOH, get-buffer is
>> idempotent too, but it seems window-state-put never receives a structure
>> with buffer objects, and I'm not sure why window-state-get should always
>> use buffer-name regardless of the value WRITABLE, i.e. why should it return
>> buffer names as strings instead of buffer objects even when WRITABLE is nil?
>
> Maybe because I didn't care about non-writable states back then.  But
> you're obviously right.  Buffers can be renamed at will and in that
> case we might have a state with a non-existent buffer or even buffers
> with names switched and therefore invalid values of start or point.

What do you think about creating new functions to convert the existing
states from non-writable to writable and back?  Then in the same session
it would be more optimal to use window states with buffer/mark objects,
and states to save to the desktop could be serialized by such functions.

> So yes: Please use a buffer object for non-writable states.

Here is a new patch:


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: window-state-prev-next-buffers.2.patch --]
[-- Type: text/x-diff, Size: 2161 bytes --]

diff --git a/lisp/window.el b/lisp/window.el
index a7318308ef..77b650fda9 100644
--- a/lisp/window.el
+++ b/lisp/window.el
@@ -5579,7 +5579,7 @@ window--state-get-1
 		(let ((point (window-point window))
 		      (start (window-start window)))
 		  `((buffer
-		     ,(buffer-name buffer)
+		     ,(if writable (buffer-name buffer) buffer)
 		     (selected . ,selected)
 		     (hscroll . ,(window-hscroll window))
 		     (fringes . ,(window-fringes window))
@@ -5599,20 +5599,20 @@ window--state-get-1
                                  (with-current-buffer buffer
                                    (copy-marker start))))))))
             ,@(when next-buffers
-                `((next-buffers . ,(mapcar (lambda (buffer)
-                                           (buffer-name buffer))
-                                         next-buffers))))
+                `((next-buffers
+                   . ,(if writable
+                          (mapcar (lambda (buffer) (buffer-name buffer))
+                                  next-buffers)
+                        next-buffers))))
             ,@(when prev-buffers
-                `((prev-buffers .
-                   ,(mapcar (lambda (entry)
-                              (list (buffer-name (nth 0 entry))
-                                    (if writable
-                                        (marker-position (nth 1 entry))
-                                      (nth 1 entry))
-                                    (if writable
-                                        (marker-position (nth 2 entry))
-                                      (nth 2 entry))))
-                            prev-buffers))))))
+                `((prev-buffers
+                   . ,(if writable
+                          (mapcar (lambda (entry)
+                                    (list (buffer-name (nth 0 entry))
+                                          (marker-position (nth 1 entry))
+                                          (marker-position (nth 2 entry))))
+                                  prev-buffers)
+                        prev-buffers))))))
 	 (tail
 	  (when (memq type '(vc hc))
 	    (let (list)

bug#32850: 27.0.50; window-swap-states doesn't swap window prev/next-buffers

[martin rudalics]

 > When testing the previous patch, I noticed that killed buffers automatically
 > disappear from prev/next-buffers lists, so there are no #<killed buffer> remains.
 > Is it handled by code in kill-buffer?

'kill-buffer' calls 'replace-buffer-in-windows' which, if the window
doesn't show the buffer, calls 'unrecord-window-buffer' which removes
the buffer from the respective lists.  But 'replace-buffer-in-windows'
handles only live windows, it can't look into window configurations.

 > What do you think about creating new functions to convert the existing
 > states from non-writable to writable and back?  Then in the same session
 > it would be more optimal to use window states with buffer/mark objects,
 > and states to save to the desktop could be serialized by such functions.

It would be interesting to have such a thing.  One could use it to
handle conversion of non-writable objects to writable ones (I always
wondered how desktop would handle non-writable values in say
'dedicated' or a window parameter) and back.  But we don't yet have a
sufficiently reliable mechanism for providing a serializable value for
a number of our objects - in particular windows and frames.

martin

bug#32850: 27.0.50; window-swap-states doesn't swap window prev/next-buffers

[Juri Linkov]

>> When testing the previous patch, I noticed that killed buffers automatically
>> disappear from prev/next-buffers lists, so there are no #<killed buffer> remains.
>> Is it handled by code in kill-buffer?
>
> 'kill-buffer' calls 'replace-buffer-in-windows' which, if the window
> doesn't show the buffer, calls 'unrecord-window-buffer' which removes
> the buffer from the respective lists.  But 'replace-buffer-in-windows'
> handles only live windows, it can't look into window configurations.

If we had a function that can look into window states, then a user
could put it on kill-buffer-hook to look into window states stored
in user variables.  It would be useful to have a function to check if
a buffer exists in window state: often there is a need not to kill
a buffer if it's stored in some window state variable (so restoring
a saved window state will always show the same buffers as before).

>> What do you think about creating new functions to convert the existing
>> states from non-writable to writable and back?  Then in the same session
>> it would be more optimal to use window states with buffer/mark objects,
>> and states to save to the desktop could be serialized by such functions.
>
> It would be interesting to have such a thing.  One could use it to
> handle conversion of non-writable objects to writable ones (I always
> wondered how desktop would handle non-writable values in say
> 'dedicated' or a window parameter) and back.  But we don't yet have a
> sufficiently reliable mechanism for providing a serializable value for
> a number of our objects - in particular windows and frames.

A serializable value of window objects are window states.  By analogy with
the eq/equal dichotomy, window objects can be compared with ‘eq’, and
window states with ‘equal’.

bug#32850: 27.0.50; window-swap-states doesn't swap window prev/next-buffers

[martin rudalics]

 > If we had a function that can look into window states, then a user
 > could put it on kill-buffer-hook to look into window states stored
 > in user variables.  It would be useful to have a function to check if
 > a buffer exists in window state: often there is a need not to kill
 > a buffer if it's stored in some window state variable (so restoring
 > a saved window state will always show the same buffers as before).

The same argument would hold for window configurations.  And looking
into those is even more difficult.

 >> But we don't yet have a
 >> sufficiently reliable mechanism for providing a serializable value for
 >> a number of our objects - in particular windows and frames.
 >
 > A serializable value of window objects are window states.  By analogy with
 > the eq/equal dichotomy, window objects can be compared with ‘eq’, and
 > window states with ‘equal’.

I meant that window objects cannot be reliably reconstructed from
states.  The more obvious reason is that a writable state can record a
buffer only via its name and buffer names will be assigned in the way
files shown by buffers are visited.  Less obvious reasons are that the
'dedicated' value or the values of window parameters may be arbitrary,
non-serializable Lisp objects you cannot reconstruct either.  So we
have no reliable bijective mappings from states to configurations and
comparing two states for equality may produce arbitrary results.

States are useful for reconstructing a configuration that existed
earlier in some approximate way as to be used by desktop and friends.
Nothing more.

martin

bug#32850: 27.0.50; window-swap-states doesn't swap window prev/next-buffers

[Juri Linkov]

> States are useful for reconstructing a configuration that existed
> earlier in some approximate way as to be used by desktop and friends.
> Nothing more.

This means that in the same session non-writable states should be used
instead of window configurations.  And for saving to the desktop file,
non-writable window states should be converted to writable window states.

bug#32850: 27.0.50; window-swap-states doesn't swap window prev/next-buffers

[martin rudalics]

 > This means that in the same session non-writable states should be used
 > instead of window configurations.

Just that the initial idea of states was that they are always
writable.

 > And for saving to the desktop file,
 > non-writable window states should be converted to writable window states.

Can you summarize why we would want non-writable states to replace
window configurations?

martin

bug#32850: 27.0.50; window-swap-states doesn't swap window prev/next-buffers

[Juri Linkov]

>> And for saving to the desktop file,
>> non-writable window states should be converted to writable window states.
>
> Can you summarize why we would want non-writable states to replace
> window configurations?

While window configurations officially are not yet deprecated,
I see no more use of them, because window configurations can't be
serialized, whereas their morally equivalent window states can.

This is because window states are data structures accessible from Lisp.
So I perceive window states as window configurations rewritten in Lisp.

bug#32850: 27.0.50; window-swap-states doesn't swap window prev/next-buffers

[martin rudalics]

 > While window configurations officially are not yet deprecated,
 > I see no more use of them, because window configurations can't be
 > serialized, whereas their morally equivalent window states can.
 >
 > This is because window states are data structures accessible from Lisp.
 > So I perceive window states as window configurations rewritten in Lisp.

'set-window-configuration' writes directly into the window structure
while 'window-state-put' splits windows instead which is much slower
and at the very least messes up window identities.

martin

bug#32850: 27.0.50; window-swap-states doesn't swap window prev/next-buffers

[Juri Linkov]

> 'set-window-configuration' writes directly into the window structure
> while 'window-state-put' splits windows instead which is much slower
> and at the very least messes up window identities.

This means that window configurations still the most suitable
within the confines of the same session.  And indeed trying to eval:

(add-hook 'window-size-change-functions
          (lambda (_) (message "window-size-change-functions called")))

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

(set-window-configuration (current-window-configuration))

you can see that window-state-put wrongly calls the hook
window-size-change-functions whereas set-window-configuration
doesn't call it that is the correct behavior.

Then we desperately need functions that will serialize window
configurations to writable window states to save them in the
desktop file.

bug#32850: 27.0.50; window-swap-states doesn't swap window prev/next-buffers

[martin rudalics]

 > Then we desperately need functions that will serialize window
 > configurations to writable window states to save them in the
 > desktop file.

What's wrong with

(save-window-excursion
   (set-window-configuration configuration)
   (window-state-get nil t))

martin

bug#32850: 27.0.50; window-swap-states doesn't swap window prev/next-buffers

[Juri Linkov]

>> Then we desperately need functions that will serialize window
>> configurations to writable window states to save them in the
>> desktop file.
>
> What's wrong with
>
> (save-window-excursion
>   (set-window-configuration configuration)
>   (window-state-get nil t))

Is it really a good solution for serialization of tens of window configurations?
Is such code suitable to be placed to desktop.el?

(let (window-configurations window-states current-window-configuration
      ;; Let-bind there hooks to nil to prevent them from running
      (window-configuration-change-hook nil)
      (window-size-change-functions nil))

  ;; As an example, populate a list of window configurations.
  ;; In reality, all these configurations are different.
  (dotimes (_ (+ 10 (random 10)))
    (push (current-window-configuration) window-configurations))

  ;; Preserve the current window configuration
  (setq current-window-configuration (current-window-configuration))
  ;; Serialize window configurations for saving in the desktop file
  (setq window-states
        (mapcar (lambda (window-configuration)
                  (save-window-excursion
                    (set-window-configuration window-configuration)
                    (window-state-get nil t)))
                window-configurations))
  ;; Restore the previous window-configuration
  (set-window-configuration current-window-configuration)
  ;; Now window-states can be saved to the desktop file

  ;; Restore window-states from the desktop file
  ;; First, preserve the current window configuration
  (setq current-window-configuration (current-window-configuration))
  ;; Restore the saved list from the desktop file
  (setq window-configurations
        (mapcar (lambda (window-state)
                  (save-window-excursion
                    (window-state-put window-state nil 'safe)
                    (current-window-configuration)))
                window-states))
  ;; Restore the previous window-configuration
  (set-window-configuration current-window-configuration)

  ;; Now window-configurations are available for another session
  window-configurations)

While testing without let-binding window-configuration-change-hook to nil,
window-configuration-change-hook was called 50 times, observable with:

(add-hook 'window-configuration-change-hook
          (lambda () (message "window-configuration-change-hook called")))

when let-bound to nil, these hooks are not called.

But after let-binding window-size-change-functions to nil,
and testing with

(add-hook 'window-size-change-functions
          (lambda (_) (message "window-size-change-functions called")))

window-size-change-functions is still called once.
I don't understand why.

bug#32850: 27.0.50; window-swap-states doesn't swap window prev/next-buffers

[martin rudalics]

 >> (save-window-excursion
 >>    (set-window-configuration configuration)
 >>    (window-state-get nil t))
 >
 > Is it really a good solution for serialization of tens of window configurations?
 > Is such code suitable to be placed to desktop.el?

Which problems do you see in practice?  I have no idea about the
internals of desktop.  If you mean that the windows' states have to be
saved too often - maybe to a backup file and as such converted from
configurations to states - we can, whenever desktop is active save a
configuration immediately to a state whenever it is stored somewhere.
That is, add some hook when a window configuration shall be saved to a
register and that register should be considered writable and saved
somewhere.  Is it that what you mean?

 > While testing without let-binding window-configuration-change-hook to nil,
 > window-configuration-change-hook was called 50 times, observable with:
 >
 > (add-hook 'window-configuration-change-hook
 >            (lambda () (message "window-configuration-change-hook called")))
 >
 > when let-bound to nil, these hooks are not called.

Obviously so.  The 'window-configuration-change-hook' mechanism is
extraordinarily dumb.

 > But after let-binding window-size-change-functions to nil,
 > and testing with
 >
 > (add-hook 'window-size-change-functions
 >            (lambda (_) (message "window-size-change-functions called")))
 >
 > window-size-change-functions is still called once.
 > I don't understand why.

'window-size-change-functions' is called by the redisplay mechanism
when it detects that at least one window has changed size.  This may
well happen outside the scope of the let binding.  If it happens when
restoring the initial configuration which should be identical to the
final one, you would have to investigate: Which window changed size
and why.  Maybe the minibuffer window is involved.

martin

bug#32850: 27.0.50; window-swap-states doesn't swap window prev/next-buffers

[Juri Linkov]

>>> (save-window-excursion
>>>    (set-window-configuration configuration)
>>>    (window-state-get nil t))
>>
>> Is it really a good solution for serialization of tens of window configurations?
>> Is such code suitable to be placed to desktop.el?
>
> Which problems do you see in practice?  I have no idea about the
> internals of desktop.  If you mean that the windows' states have to be
> saved too often -

Yes, too often - according to desktop-auto-save-timeout, it should do
this juggling with window configurations and states every 30 seconds.

> maybe to a backup file and as such converted from configurations to
> states - we can, whenever desktop is active save a configuration
> immediately to a state whenever it is stored somewhere.  That is, add
> some hook when a window configuration shall be saved to a register and
> that register should be considered writable and saved somewhere.
> Is it that what you mean?

In fact this means maintaining a duplicate data structure,
i.e. in parallel to keep in one list - window configurations,
but in another list - window states.  The downside is data duplication.
If this is the only available solution, then it's ok.

But the problem is that window configurations can't be used
even in the same session, because they don't keep prev/next-buffers.
Please try to eval:

(defun set-buffers ()
  (let* ((buffer-a (get-buffer-create "a"))
         (marker-a (set-marker (make-marker) 1 buffer-a))
         (buffer-b (get-buffer-create "b"))
         (marker-b (set-marker (make-marker) 1 buffer-b)))
    (set-window-prev-buffers nil (list (list buffer-a marker-a marker-a)
                                       (list buffer-b marker-b marker-b)))
    (set-window-next-buffers nil (list buffer-a))))

(progn (set-buffers)
       (let ((window-state (window-state-get nil t)))
         ;; Reset prev/next-buffers
         (set-window-prev-buffers nil nil)
         (set-window-next-buffers nil nil)
         ;; Restore old state with preserved prev/next-buffers
         (window-state-put window-state nil 'safe)
	 (list (window-prev-buffers) (window-next-buffers))))

window-state-put in the above code correctly restores the old values of
prev/next-buffers kept in window-state.

(progn (set-buffers)
       ;; Remember window-configuration with prev/next-buffers
       (let ((window-configuration (current-window-configuration)))
         ;; Reset prev/next-buffers
         (set-window-prev-buffers nil nil)
         (set-window-next-buffers nil nil)
         ;; This doesn't restore remembered prev/next-buffers
         (set-window-configuration window-configuration)
         ;; window-prev-buffers and window-next-buffers are still nil
	 (list (window-prev-buffers) (window-next-buffers))))

But unfortunately set-window-configuration doesn't restore
the old values of prev/next-buffers.

bug#32850: 27.0.50; window-swap-states doesn't swap window prev/next-buffers

[martin rudalics]

 >> Which problems do you see in practice?  I have no idea about the
 >> internals of desktop.  If you mean that the windows' states have to be
 >> saved too often -
 >
 > Yes, too often - according to desktop-auto-save-timeout, it should do
 > this juggling with window configurations and states every 30 seconds.

That's not reasonable.  More often than not this will catch the
current window configuration in an inconsistent state where the
minibuffer is active or the user is in a window excurstion.

But does desktop save window configurations or states stored somewhere
in the first place?

 > In fact this means maintaining a duplicate data structure,
 > i.e. in parallel to keep in one list - window configurations,
 > but in another list - window states.  The downside is data duplication.
 > If this is the only available solution, then it's ok.
 >
 > But the problem is that window configurations can't be used
 > even in the same session, because they don't keep prev/next-buffers.
...
 > But unfortunately set-window-configuration doesn't restore
 > the old values of prev/next-buffers.

That's disputable anyway.  When, in a window excursion, you show some
buffer in a window, don't you want to record that buffer in that
window's list of previous buffers after exiting from the excursion?

Anyway.  It would be tedious but probably not impossible to provide a
function that transforms a configuration into a state.  Doing the
opposite is conceptually questionable, at the very least.

A basic invariant of the windows code is that a valid window cannot
appear twice on the same or a different frame.  It may, however,
appear an arbitrary number of times in stored window configurations.

One consequence implied by that invariant is that you can restore a
window configuration only into the frame from where you saved it
earlier.  When a frame gets deleted, all configurations drawn earlier
from that frame are virtually lost.

martin

bug#32850: 27.0.50; window-swap-states doesn't swap window prev/next-buffers

[Eli Zaretskii]

> Date: Fri, 26 Oct 2018 09:39:25 +0200
> From: martin rudalics <rudalics@gmx.at>
> Cc: 32850@debbugs.gnu.org
> 
>  >> Which problems do you see in practice?  I have no idea about the
>  >> internals of desktop.  If you mean that the windows' states have to be
>  >> saved too often -
>  >
>  > Yes, too often - according to desktop-auto-save-timeout, it should do
>  > this juggling with window configurations and states every 30 seconds.
> 
> That's not reasonable.

I agree.  Which is why I have desktop-auto-save-timeout customized to
nil on my main machine, and to very large values on others.  When this
feature first came out, I found that it frequently causes delays in
Emacs responses, and I don't see any reason to have the desktop saved
every 30 sec.

bug#32850: 27.0.50; window-swap-states doesn't swap window prev/next-buffers

[Juri Linkov]

>> But unfortunately set-window-configuration doesn't restore
>> the old values of prev/next-buffers.
>
> That's disputable anyway.  When, in a window excursion, you show some
> buffer in a window, don't you want to record that buffer in that
> window's list of previous buffers after exiting from the excursion?

Thanks for the hint, I see now that the problem is not in
set-window-configuration, but in window-state-put.

In case of set-window-configuration, `(set-window-next-buffers nil nil)'
correctly changes the value of window-next-buffers in the same
window that was stored in window-configuration and that is
currently selected, i.e. this produces the right result
when prev/next-buffers are modified in a new window
(so not in the window shared in window configurations):

(progn (set-buffers)
       ;; Remember window-configuration with prev/next-buffers
       (let ((window-configuration (current-window-configuration)))
         ;; Create a new window
         (delete-other-windows) (split-window) (delete-window)
         ;; Reset prev/next-buffers
         (set-window-prev-buffers nil nil)
         (set-window-next-buffers nil nil)
         ;; This correctly restores remembered prev/next-buffers
         ;; because window-next-buffers are modified in another window
         (set-window-configuration window-configuration)
         ;; window-prev-buffers and window-next-buffers are correct
	 (list (window-prev-buffers) (window-next-buffers))))

But window-state-put has such deficiency with its undocumented
prerequisite that a new unique window should be created manually
before calling window-state-put to put the state into a new
window (to not share it with other window configurations).
This means a need in such ugly hack:

;; Create a new window
(delete-other-windows) (split-window) (delete-window)
;; Put a previous state into the new window
;; to not share it with other window configurations
(window-state-put previously-saved-window-state nil 'safe)
;; Get its window configuration
(current-window-configuration)

Please confirm is `(delete-other-windows) (split-window) (delete-window)'
is the right idiom to create a new window to put the window state into,
or there is a special function to create a window, that I can't find.

> Anyway.  It would be tedious but probably not impossible to provide a
> function that transforms a configuration into a state.  Doing the
> opposite is conceptually questionable, at the very least.

The code above does the opposite, but it requires an ugly workaround
for window-state-put.

> A basic invariant of the windows code is that a valid window cannot
> appear twice on the same or a different frame.  It may, however,
> appear an arbitrary number of times in stored window configurations.

Like the earlier code snippet demonstrates it causes problems when
the same window appears an arbitrary number of times in stored window
configurations.  Thus the need to create a new window to put the state into
(currently window-state-put itself doesn't create a new unique window).

bug#32850: 27.0.50; window-swap-states doesn't swap window prev/next-buffers

[martin rudalics]

 > But window-state-put has such deficiency with its undocumented
 > prerequisite that a new unique window should be created manually
 > before calling window-state-put to put the state into a new
 > window (to not share it with other window configurations).

Maybe "to not share it with other frames" would be more precise.  What
you probably want is a facility to clone a window.  I have no idea how
to do that and which semantics to attach to a cloned window.  Always
keep in mind: Windows that have been deleted but go on "living" in
stored configurations are zombies.  And dealing with the undead can be
cumbersome ...

 > This means a need in such ugly hack:
 >
 > ;; Create a new window
 > (delete-other-windows) (split-window) (delete-window)
 > ;; Put a previous state into the new window
 > ;; to not share it with other window configurations
 > (window-state-put previously-saved-window-state nil 'safe)
 > ;; Get its window configuration
 > (current-window-configuration)
 >
 > Please confirm is `(delete-other-windows) (split-window) (delete-window)'
 > is the right idiom to create a new window to put the window state into,
 > or there is a special function to create a window, that I can't find.

You can put a window state into any valid window, usually the frame's
root window.  Just that under the hood 'window-state-put' will do the
'delete-other-windows' then.

 > The code above does the opposite, but it requires an ugly workaround
 > for window-state-put.

Please elaborate.  In which sense does that code make a configuration
from a state?

 > Like the earlier code snippet demonstrates it causes problems when
 > the same window appears an arbitrary number of times in stored window
 > configurations.  Thus the need to create a new window to put the state into
 > (currently window-state-put itself doesn't create a new unique window).

I lost you.  Anything Lisp code can do is create a new unique window.
Lisp code cannot create a new non-unique window.

martin

bug#32850: 27.0.50; window-swap-states doesn't swap window prev/next-buffers

[Juri Linkov]

> What you probably want is a facility to clone a window.  I have no
> idea how to do that and which semantics to attach to a cloned window.

I guess a cloned window implies the same state with a different
identity.

> You can put a window state into any valid window, usually the frame's
> root window.

I see no difference, still the same problem is that
window-state-put doesn't create a new window:

(let* ((orig-window (catch 'live
                      (walk-window-subtree
                       (lambda (window)
                         (when (window-live-p window)
                           (throw 'live window)))
                       (frame-root-window (selected-frame)))))
       (orig-window-state (window-state-get (frame-root-window (selected-frame)) 'writable)))
  (window-state-put orig-window-state (frame-root-window (selected-frame)) 'safe)
  (eq orig-window (catch 'live
                    (walk-window-subtree
                     (lambda (window)
                       (when (window-live-p window)
                         (throw 'live window)))
                     (frame-root-window (selected-frame))))))

returns t that indicates the problem.

Thus such ugly hack is required to workaround this problem:

(let* ((orig-window (catch 'live
                      (walk-window-subtree
                       (lambda (window)
                         (when (window-live-p window)
                           (throw 'live window)))
                       (frame-root-window (selected-frame)))))
       (orig-window-state (window-state-get (frame-root-window (selected-frame)) 'writable)))

  ;; Hack alert!  Create a new window
  (delete-other-windows) (split-window) (delete-window)

  (window-state-put orig-window-state (frame-root-window (selected-frame)) 'safe)
  (eq orig-window (catch 'live
                    (walk-window-subtree
                     (lambda (window)
                       (when (window-live-p window)
                         (throw 'live window)))
                     (frame-root-window (selected-frame))))))

correctly returns nil.

> Just that under the hood 'window-state-put' will do the
> 'delete-other-windows' then.

Now I see the root of this problem: it's because 'delete-other-windows'
keeps one window and reuses it for the restored state.

So a proper fix for 'window-state-put' should use a cloned window
like you proposed.

bug#32850: 27.0.50; window-swap-states doesn't swap window prev/next-buffers

[martin rudalics]

 >> What you probably want is a facility to clone a window.  I have no
 >> idea how to do that and which semantics to attach to a cloned window.
 >
 > I guess a cloned window implies the same state with a different
 > identity.

A different window object, hence.

 > Now I see the root of this problem: it's because 'delete-other-windows'
 > keeps one window and reuses it for the restored state.

There's no reasonable way to avoid that - each frame needs a live root
window.

 > So a proper fix for 'window-state-put' should use a cloned window
 > like you proposed.

Couldn't we work around this problem some other way?  Pumping up the
window's sequence number, for example?  Adding a function that
replaces the live root window of a frame with a different object is
somewhat ludicrous.

martin

bug#32850: 27.0.50; window-swap-states doesn't swap window prev/next-buffers

[Juri Linkov]

>> So a proper fix for 'window-state-put' should use a cloned window
>> like you proposed.
>
> Couldn't we work around this problem some other way?

I proposed one workaround:

  (delete-other-windows) (split-window) (delete-window)

> Pumping up the window's sequence number, for example?

What will 'eq' return on windows with different sequence numbers?

bug#32850: 27.0.50; window-swap-states doesn't swap window prev/next-buffers

[martin rudalics]

 > I proposed one workaround:
 >
 >    (delete-other-windows) (split-window) (delete-window)

OK.  If we document it sufficiently, let's use that.

 >> Pumping up the window's sequence number, for example?
 >
 > What will 'eq' return on windows with different sequence numbers?

t if it is the same window object.  I obviously meant the caller to
check the sequence number in addition to 'eq'.

martin

bug#32850: 27.0.50; window-swap-states doesn't swap window prev/next-buffers

[Juri Linkov]

>>    (delete-other-windows) (split-window) (delete-window)
>
> OK.  If we document it sufficiently, let's use that.

I thought it's ugly.  But if there is no other way:

diff --git a/lisp/window.el b/lisp/window.el
index bcd4fa2959..9fdf005202 100644
--- a/lisp/window.el
+++ b/lisp/window.el
@@ -5898,20 +5898,12 @@ window-state-put
 windows can get as small as `window-safe-min-height' and
 `window-safe-min-width'."
   (setq window-state-put-stale-windows nil)
-  (setq window (window-normalize-window window))
 
-  ;; When WINDOW is internal, reduce it to a live one to put STATE into,
-  ;; see Bug#16793.
+  ;; When WINDOW is internal or nil, create a new window.
   (unless (window-live-p window)
-    (let ((root window))
-      (setq window (catch 'live
-                     (walk-window-subtree
-                      (lambda (window)
-                        (when (and (window-live-p window)
-                                   (not (window-parameter window 'window-side)))
-                          (throw 'live window)))
-                      root)))
-      (delete-other-windows-internal window root)))
+    (delete-other-windows)
+    (setq window (split-window))
+    (delete-window))
 
   (set-window-dedicated-p window nil)
 

bug#32850: 27.0.50; window-swap-states doesn't swap window prev/next-buffers

[martin rudalics]

 >>>     (delete-other-windows) (split-window) (delete-window)
 >>
 >> OK.  If we document it sufficiently, let's use that.
 >
 > I thought it's ugly.  But if there is no other way:

It's very ugly.

 > diff --git a/lisp/window.el b/lisp/window.el
 > index bcd4fa2959..9fdf005202 100644
 > --- a/lisp/window.el
 > +++ b/lisp/window.el
 > @@ -5898,20 +5898,12 @@ window-state-put
 >   windows can get as small as `window-safe-min-height' and
 >   `window-safe-min-width'."
 >     (setq window-state-put-stale-windows nil)
 > -  (setq window (window-normalize-window window))
 >
 > -  ;; When WINDOW is internal, reduce it to a live one to put STATE into,
 > -  ;; see Bug#16793.
 > +  ;; When WINDOW is internal or nil, create a new window.
 >     (unless (window-live-p window)
 > -    (let ((root window))
 > -      (setq window (catch 'live
 > -                     (walk-window-subtree
 > -                      (lambda (window)
 > -                        (when (and (window-live-p window)
 > -                                   (not (window-parameter window 'window-side)))
 > -                          (throw 'live window)))
 > -                      root)))
 > -      (delete-other-windows-internal window root)))

So you want to elide this part because 'delete-other-windows' below
handles it?  But the latter respects the 'no-delete-other-windows'
parameter too which might be unwanted in the state putting mechanism.

 > +    (delete-other-windows)
 > +    (setq window (split-window))
 > +    (delete-window))

martin

bug#32850: 27.0.50; window-swap-states doesn't swap window prev/next-buffers

[Juri Linkov]

> So you want to elide this part because 'delete-other-windows' below
> handles it?  But the latter respects the 'no-delete-other-windows'
> parameter too which might be unwanted in the state putting mechanism.

Then is this right?

diff --git a/lisp/window.el b/lisp/window.el
index bcd4fa2959..346096c2f8 100644
--- a/lisp/window.el
+++ b/lisp/window.el
@@ -5898,20 +5898,12 @@ window-state-put
 windows can get as small as `window-safe-min-height' and
 `window-safe-min-width'."
   (setq window-state-put-stale-windows nil)
-  (setq window (window-normalize-window window))
 
-  ;; When WINDOW is internal, reduce it to a live one to put STATE into,
-  ;; see Bug#16793.
+  ;; When WINDOW is internal or nil, create a new window.
   (unless (window-live-p window)
-    (let ((root window))
-      (setq window (catch 'live
-                     (walk-window-subtree
-                      (lambda (window)
-                        (when (and (window-live-p window)
-                                   (not (window-parameter window 'window-side)))
-                          (throw 'live window)))
-                      root)))
-      (delete-other-windows-internal window root)))
+    (delete-other-windows-internal)
+    (setq window (split-window))
+    (delete-window))
 
   (set-window-dedicated-p window nil)
 

bug#32850: 27.0.50; window-swap-states doesn't swap window prev/next-buffers

[martin rudalics]

 > Then is this right?
 >
 > diff --git a/lisp/window.el b/lisp/window.el
 > index bcd4fa2959..346096c2f8 100644
 > --- a/lisp/window.el
 > +++ b/lisp/window.el
 > @@ -5898,20 +5898,12 @@ window-state-put
 >   windows can get as small as `window-safe-min-height' and
 >   `window-safe-min-width'."
 >     (setq window-state-put-stale-windows nil)
 > -  (setq window (window-normalize-window window))
 >
 > -  ;; When WINDOW is internal, reduce it to a live one to put STATE into,
 > -  ;; see Bug#16793.
 > +  ;; When WINDOW is internal or nil, create a new window.
 >     (unless (window-live-p window)
 > -    (let ((root window))
 > -      (setq window (catch 'live
 > -                     (walk-window-subtree
 > -                      (lambda (window)
 > -                        (when (and (window-live-p window)
 > -                                   (not (window-parameter window 'window-side)))
 > -                          (throw 'live window)))
 > -                      root)))
 > -      (delete-other-windows-internal window root)))
 > +    (delete-other-windows-internal)
 > +    (setq window (split-window))
 > +    (delete-window))
 >
 >     (set-window-dedicated-p window nil)

What would be bad about leaving most of that code in place and just
adding the

 > +    (setq window (split-window))
 > +    (delete-window))

ugliness?

martin

bug#32850: 27.0.50; window-swap-states doesn't swap window prev/next-buffers

[Juri Linkov]

>> -    (let ((root window))
>> -      (setq window (catch 'live
>> -                     (walk-window-subtree
>> -                      (lambda (window)
>> -                        (when (and (window-live-p window)
>> -                                   (not (window-parameter window 'window-side)))
>> -                          (throw 'live window)))
>> -                      root)))
>> -      (delete-other-windows-internal window root)))
>> +    (delete-other-windows-internal)
>> +    (setq window (split-window))
>> +    (delete-window))
>>
>>     (set-window-dedicated-p window nil)
>
> What would be bad about leaving most of that code in place and just
> adding the
>
>> +    (setq window (split-window))
>> +    (delete-window))

There is no need to find a window using walk-window-subtree
when this window will be deleted anyway (if function argument
is nil or not a live window).

OTOH, a live window should not be deleted when given
by the argument of window-state-put.

bug#32850: 27.0.50; window-swap-states doesn't swap window prev/next-buffers

[martin rudalics]

 > There is no need to find a window using walk-window-subtree
 > when this window will be deleted anyway (if function argument
 > is nil or not a live window).
 >
 > OTOH, a live window should not be deleted when given
 > by the argument of window-state-put.

Sounds reasonable (Bug#16793 just wants a live window there and
splitting and deleting the old one should do that trivially).  Feel
free to install if you see no further problems.

martin

bug#32850: 27.0.50; window-swap-states doesn't swap window prev/next-buffers

[Juri Linkov]

> Sounds reasonable (Bug#16793 just wants a live window there and
> splitting and deleting the old one should do that trivially).  Feel
> free to install if you see no further problems.

After reading bug#16793, I see that the original intention was
to restore a window state on the same frame where the subtree of
the specified internal window is.  If this is important, then
a better change is:

diff --git a/doc/lispref/windows.texi b/doc/lispref/windows.texi
index 772bcdf9a6..49698f45bf 100644
--- a/doc/lispref/windows.texi
+++ b/doc/lispref/windows.texi
@@ -5706,9 +5706,10 @@ Window Configurations
 The argument @var{state} should be the state of a window returned by
 an earlier invocation of @code{window-state-get}, see above.  The
 optional argument @var{window} can be either a live window or an
-internal window (@pxref{Windows and Frames}) and defaults to the
-selected one.  If @var{window} is not live, it is replaced by a live
-window before putting @var{state} into it.
+internal window (@pxref{Windows and Frames}).  If @var{window} is not
+live, it is replaced by a new live window created on the same subtree
+before putting @var{state} into it.  If @var{window} is nil, it puts
+the window state into a new window.
 
 If the optional argument @var{ignore} is non-@code{nil}, it means to ignore
 minimum window sizes and fixed-size restrictions.  If @var{ignore}

diff --git a/lisp/window.el b/lisp/window.el
index bcd4fa2959..b8a8b81af3 100644
--- a/lisp/window.el
+++ b/lisp/window.el
@@ -5889,29 +5889,34 @@ window-state-put
   "Put window state STATE into WINDOW.
 STATE should be the state of a window returned by an earlier
 invocation of `window-state-get'.  Optional argument WINDOW must
-specify a valid window and defaults to the selected one.  If
-WINDOW is not live, replace WINDOW by a live one before putting
-STATE into it.
+specify a valid window.  If WINDOW is not live, replace WINDOW
+by a new live one created on the same subtree.  If WINDOW is nil,
+create a new window before putting STATE into it.
 
 Optional argument IGNORE non-nil means ignore minimum window
 sizes and fixed size restrictions.  IGNORE equal `safe' means
 windows can get as small as `window-safe-min-height' and
 `window-safe-min-width'."
   (setq window-state-put-stale-windows nil)
-  (setq window (window-normalize-window window))
 
-  ;; When WINDOW is internal, reduce it to a live one to put STATE into,
-  ;; see Bug#16793.
+  ;; When WINDOW is internal or nil, reduce it to a live one,
+  ;; then create a new window on the same subtree to put STATE into.
   (unless (window-live-p window)
     (let ((root window))
-      (setq window (catch 'live
-                     (walk-window-subtree
-                      (lambda (window)
-                        (when (and (window-live-p window)
-                                   (not (window-parameter window 'window-side)))
-                          (throw 'live window)))
-                      root)))
-      (delete-other-windows-internal window root)))
+      (setq window (if root
+                       (catch 'live
+                         (walk-window-subtree
+                          (lambda (window)
+                            (when (and (window-live-p window)
+                                       (not (window-parameter
+                                             window 'window-side)))
+                              (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)))))
 
   (set-window-dedicated-p window nil)
 

bug#32850: 27.0.50; window-swap-states doesn't swap window prev/next-buffers

[Eli Zaretskii]

> From: Juri Linkov <juri@linkov.net>
> Date: Mon, 05 Nov 2018 23:38:27 +0200
> Cc: 32850@debbugs.gnu.org
> 
> diff --git a/doc/lispref/windows.texi b/doc/lispref/windows.texi
> index 772bcdf9a6..49698f45bf 100644
> --- a/doc/lispref/windows.texi
> +++ b/doc/lispref/windows.texi
> @@ -5706,9 +5706,10 @@ Window Configurations
>  The argument @var{state} should be the state of a window returned by
>  an earlier invocation of @code{window-state-get}, see above.  The
>  optional argument @var{window} can be either a live window or an
> -internal window (@pxref{Windows and Frames}) and defaults to the
> -selected one.  If @var{window} is not live, it is replaced by a live
> -window before putting @var{state} into it.
> +internal window (@pxref{Windows and Frames}).  If @var{window} is not
                                                  ^^^^^^^^^^^^^^^^^^^^^^
> +live, it is replaced by a new live window created on the same subtree
   ^^^^

"If @var{window} is not a live window, ..."

> +before putting @var{state} into it.  If @var{window} is nil, it puts
                                                           ^^^
"nil" is a symbol, so @code{nil}.

> diff --git a/lisp/window.el b/lisp/window.el
> index bcd4fa2959..b8a8b81af3 100644
> --- a/lisp/window.el
> +++ b/lisp/window.el
> @@ -5889,29 +5889,34 @@ window-state-put
>    "Put window state STATE into WINDOW.
>  STATE should be the state of a window returned by an earlier
>  invocation of `window-state-get'.  Optional argument WINDOW must
> -specify a valid window and defaults to the selected one.  If
> -WINDOW is not live, replace WINDOW by a live one before putting
> -STATE into it.
> +specify a valid window.  If WINDOW is not live, replace WINDOW

Same here.

bug#32850: 27.0.50; window-swap-states doesn't swap window prev/next-buffers

[Juri Linkov]

>> -selected one.  If @var{window} is not live, it is replaced by a live
>> -window before putting @var{state} into it.
>> +internal window (@pxref{Windows and Frames}).  If @var{window} is not
>                                                   ^^^^^^^^^^^^^^^^^^^^^^
>> +live, it is replaced by a new live window created on the same subtree
>    ^^^^
>
> "If @var{window} is not a live window, ..."
>
>> +before putting @var{state} into it.  If @var{window} is nil, it puts
>                                                            ^^^
> "nil" is a symbol, so @code{nil}.
>
>> -specify a valid window and defaults to the selected one.  If
>> -WINDOW is not live, replace WINDOW by a live one before putting
>> -STATE into it.
>> +specify a valid window.  If WINDOW is not live, replace WINDOW
>
> Same here.

Fixed in a new patch, also taking into account Martin's suggestion
to not use the term "subtree".

diff --git a/doc/lispref/windows.texi b/doc/lispref/windows.texi
index 772bcdf9a6..9301fdfa9d 100644
--- a/doc/lispref/windows.texi
+++ b/doc/lispref/windows.texi
@@ -5706,9 +5706,10 @@ Window Configurations
 The argument @var{state} should be the state of a window returned by
 an earlier invocation of @code{window-state-get}, see above.  The
 optional argument @var{window} can be either a live window or an
-internal window (@pxref{Windows and Frames}) and defaults to the
-selected one.  If @var{window} is not live, it is replaced by a live
-window before putting @var{state} into it.
+internal window (@pxref{Windows and Frames}).  If @var{window} is not
+a live window, it is replaced by a new live window created on the same
+frame before putting @var{state} into it.  If @var{window} is @code{nil},
+it puts the window state into a new window.
 
 If the optional argument @var{ignore} is non-@code{nil}, it means to ignore
 minimum window sizes and fixed-size restrictions.  If @var{ignore}
diff --git a/lisp/window.el b/lisp/window.el
index bcd4fa2959..c0eeba7261 100644
--- a/lisp/window.el
+++ b/lisp/window.el
@@ -5889,29 +5889,34 @@ window-state-put
   "Put window state STATE into WINDOW.
 STATE should be the state of a window returned by an earlier
 invocation of `window-state-get'.  Optional argument WINDOW must
-specify a valid window and defaults to the selected one.  If
-WINDOW is not live, replace WINDOW by a live one before putting
-STATE into it.
+specify a valid window.  If WINDOW is not a live window,
+replace WINDOW by a new live window created on the same frame.
+If WINDOW is nil, create a new window before putting STATE into it.
 
 Optional argument IGNORE non-nil means ignore minimum window
 sizes and fixed size restrictions.  IGNORE equal `safe' means
 windows can get as small as `window-safe-min-height' and
 `window-safe-min-width'."
   (setq window-state-put-stale-windows nil)
-  (setq window (window-normalize-window window))
 
-  ;; When WINDOW is internal, reduce it to a live one to put STATE into,
-  ;; see Bug#16793.
+  ;; When WINDOW is internal or nil, reduce it to a live one,
+  ;; then create a new window on the same frame to put STATE into.
   (unless (window-live-p window)
     (let ((root window))
-      (setq window (catch 'live
-                     (walk-window-subtree
-                      (lambda (window)
-                        (when (and (window-live-p window)
-                                   (not (window-parameter window 'window-side)))
-                          (throw 'live window)))
-                      root)))
-      (delete-other-windows-internal window root)))
+      (setq window (if root
+                       (catch 'live
+                         (walk-window-subtree
+                          (lambda (window)
+                            (when (and (window-live-p window)
+                                       (not (window-parameter
+                                             window 'window-side)))
+                              (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)))))
 
   (set-window-dedicated-p window nil)
 

bug#32850: 27.0.50; window-swap-states doesn't swap window prev/next-buffers

[martin rudalics]

 > +internal window (@pxref{Windows and Frames}).  If @var{window} is not
 > +live, it is replaced by a new live window created on the same subtree

The term "subtree" seems hardly comprehensible in this context.

martin

bug#32850: 27.0.50; window-swap-states doesn't swap window prev/next-buffers

[Juri Linkov]

> But does desktop save window configurations or states stored somewhere
> in the first place?

In case you're interested, here's is a working solution
that saves window configurations to the desktop file:
https://gitlab.com/link0ff/emacs-wincows/commit/e2d1c132

bug#32850: 27.0.50; window-swap-states doesn't swap window prev/next-buffers

[martin rudalics]

 > In case you're interested, here's is a working solution
 > that saves window configurations to the desktop file:
 > https://gitlab.com/link0ff/emacs-wincows/commit/e2d1c132

It would be nice to combine wincows.el with frame-tabs.el, somehow.

martin

bug#32850: 27.0.50; window-swap-states doesn't swap window prev/next-buffers

[Juri Linkov]

>> In case you're interested, here's is a working solution
>> that saves window configurations to the desktop file:
>> https://gitlab.com/link0ff/emacs-wincows/commit/e2d1c132
>
> It would be nice to combine wincows.el with frame-tabs.el, somehow.

Since now finally we have persistence for window configurations,
I'm going to revive the x-tabs branch.  The package wincows.el
was needed for testing what will be the internal functionality
in the x-tabs branch.  And for the user interface it could use
what you implemented for displaying tabs in side windows in frame-tabs.el.
Would you allow using your code from frame-tabs.el in the x-tabs branch?

bug#32850: 27.0.50; window-swap-states doesn't swap window prev/next-buffers

[martin rudalics]

 > Since now finally we have persistence for window configurations,
 > I'm going to revive the x-tabs branch.  The package wincows.el
 > was needed for testing what will be the internal functionality
 > in the x-tabs branch.  And for the user interface it could use
 > what you implemented for displaying tabs in side windows in frame-tabs.el.
 > Would you allow using your code from frame-tabs.el in the x-tabs branch?

Sure.  IIRC you used a menubar-like window to implement a tab-bar back
then.  Does the "x" stand for X-only?  Would this limit us to X-based
builds?

martin

bug#32850: 27.0.50; window-swap-states doesn't swap window prev/next-buffers

[Juri Linkov]

>> Since now finally we have persistence for window configurations,
>> I'm going to revive the x-tabs branch.  The package wincows.el
>> was needed for testing what will be the internal functionality
>> in the x-tabs branch.  And for the user interface it could use
>> what you implemented for displaying tabs in side windows in frame-tabs.el.
>> Would you allow using your code from frame-tabs.el in the x-tabs branch?
>
> Sure.  IIRC you used a menubar-like window to implement a tab-bar back
> then.  Does the "x" stand for X-only?  Would this limit us to X-based
> builds?

The x-tabs branch is for X-based builds, and the gtk-tabs branch
that Jan implemented is for GTK-based builds.  Both are for the
frame-local tab-bars and can't be used for window-local tab-bars.
Do you know a solution for window-local tab-bars other than
duplicating header-line in each window to provide shelter for tab-bars?

bug#32850: 27.0.50; window-swap-states doesn't swap window prev/next-buffers

[martin rudalics]

 > The x-tabs branch is for X-based builds, and the gtk-tabs branch
 > that Jan implemented is for GTK-based builds.  Both are for the
 > frame-local tab-bars and can't be used for window-local tab-bars.
 > Do you know a solution for window-local tab-bars other than
 > duplicating header-line in each window to provide shelter for tab-bars?

Mine uses atomic windows.  But window-local tab-bars are not really
useful for displaying window configurations anyway (just like
mode-lines are not suited for displaying the battery charge IMHO).

Anyway, the problem with an X-based solution is that we would have to
find a solution for non-X-based systems.  How "much" X-dependency does
x-tabs have?

martin

bug#32850: 27.0.50; window-swap-states doesn't swap window prev/next-buffers

[Juri Linkov]

>> The x-tabs branch is for X-based builds, and the gtk-tabs branch
>> that Jan implemented is for GTK-based builds.  Both are for the
>> frame-local tab-bars and can't be used for window-local tab-bars.
>> Do you know a solution for window-local tab-bars other than
>> duplicating header-line in each window to provide shelter for tab-bars?
>
> Mine uses atomic windows.

I thought side windows, no?

> But window-local tab-bars are not really useful for displaying window
> configurations anyway

Yes, they are not useful for window configurations, but
window-local tab-bars are extremely useful for displaying
the list of prev/next buffers as tabs.  So the user can see
the names of all buffers that were displayed in the window,
and clicking on the tab will switch to any of previously
displayed buffers.  And clicking on [x] will remove the
buffer from the list of previous window-local buffers.

> (just like mode-lines are not suited for displaying the battery charge
> IMHO).

I agree, mode-lines duplicate the battery charge information
in every window.  What do you think about creating the frame-local
mode-line using the bottom side window?

> Anyway, the problem with an X-based solution is that we would have to
> find a solution for non-X-based systems.  How "much" X-dependency does
> x-tabs have?

The same X-dependency as the tool-bar uses, and there is no tool-bar
on non-X-based systems.

bug#32850: 27.0.50; window-swap-states doesn't swap window prev/next-buffers

[martin rudalics]

 >>> The x-tabs branch is for X-based builds, and the gtk-tabs branch
 >>> that Jan implemented is for GTK-based builds.  Both are for the
 >>> frame-local tab-bars and can't be used for window-local tab-bars.
 >>> Do you know a solution for window-local tab-bars other than
 >>> duplicating header-line in each window to provide shelter for tab-bars?
 >>
 >> Mine uses atomic windows.
 >
 > I thought side windows, no?

Atomic windows.  Side windows are used for the frame-local tab-bar.

 >> But window-local tab-bars are not really useful for displaying window
 >> configurations anyway
 >
 > Yes, they are not useful for window configurations, but
 > window-local tab-bars are extremely useful for displaying
 > the list of prev/next buffers as tabs.  So the user can see
 > the names of all buffers that were displayed in the window,
 > and clicking on the tab will switch to any of previously
 > displayed buffers.  And clicking on [x] will remove the
 > buffer from the list of previous window-local buffers.

That was my idea behind window-local tab-bars back then.  Today I
think that we should instead pop up a menu of previous buffers when
moving the mouse over the mode-line's buffer-name.

 > I agree, mode-lines duplicate the battery charge information
 > in every window.  What do you think about creating the frame-local
 > mode-line using the bottom side window?

By default I would display them (possibly right-adjusted) only in the
mode-line of the bottom-most-right-most window.

martin

bug#32850: 27.0.50; window-swap-states doesn't swap window prev/next-buffers

[Juri Linkov]

>> window-local tab-bars are extremely useful for displaying
>> the list of prev/next buffers as tabs.  So the user can see
>> the names of all buffers that were displayed in the window,
>> and clicking on the tab will switch to any of previously
>> displayed buffers.  And clicking on [x] will remove the
>> buffer from the list of previous window-local buffers.
>
> That was my idea behind window-local tab-bars back then.

That was a very good idea.

> Today I think that we should instead pop up a menu of previous buffers
> when moving the mouse over the mode-line's buffer-name.

Not intuitive at all.

>> I agree, mode-lines duplicate the battery charge information
>> in every window.  What do you think about creating the frame-local
>> mode-line using the bottom side window?
>
> By default I would display them (possibly right-adjusted) only in the
> mode-line of the bottom-most-right-most window.

Good idea.  Simply to add a conditional display to mode-line-format with

  (and (not (window-in-direction 'down))
       (not (window-in-direction 'left)))

bug#32850: 27.0.50; window-swap-states doesn't swap window prev/next-buffers

[martin rudalics]

 >> Today I think that we should instead pop up a menu of previous buffers
 >> when moving the mouse over the mode-line's buffer-name.
 >
 > Not intuitive at all.

Why not?  We currently tell there in a tooltip that it's a "buffer
name" and how to switch to the previous and next buffer.  Is that
intuitive?

 > Good idea.  Simply to add a conditional display to mode-line-format with
 >
 >    (and (not (window-in-direction 'down))
 >         (not (window-in-direction 'left)))

Right-adjusted would be fine to make it more clear that this is not a
window-buffer pertinent information.

martin

bug#32850: 27.0.50; window-swap-states doesn't swap window prev/next-buffers

[Juri Linkov]

>>> Today I think that we should instead pop up a menu of previous buffers
>>> when moving the mouse over the mode-line's buffer-name.
>>
>> Not intuitive at all.
>
> Why not?  We currently tell there in a tooltip that it's a "buffer
> name" and how to switch to the previous and next buffer.  Is that
> intuitive?

I don't use the mouse.  Whereas window-local tab-bar showing a list
of prev/next-buffers allows using the keyboard to navigate.
For example, when the window-local tab-bar shows such tabs:

[buffer-2] [buffer-1] [current-buffer] [buffer+1] [buffer+2]

Then I know that I can just type `C-2 C-x left' to get to the second
previous buffer, or to get the second next buffer with `C-2 C-x right'.

>> Good idea.  Simply to add a conditional display to mode-line-format with
>>
>>    (and (not (window-in-direction 'down))
>>         (not (window-in-direction 'left)))
>
> Right-adjusted would be fine to make it more clear that this is not a
> window-buffer pertinent information.

If there is enough space to put global information.  For example, in Gnus
mode-lines are so long, that have no free space for more information.

> I forgot.  You probably mean
>
>    (and (not (window-in-direction 'down nil nil nil nil 'nomini))
>         (not (window-in-direction 'right nil nil nil nil 'nomini)))
>
> here.

Exactly.

bug#32850: 27.0.50; window-swap-states doesn't swap window prev/next-buffers

[martin rudalics]

 > I don't use the mouse.  Whereas window-local tab-bar showing a list
 > of prev/next-buffers allows using the keyboard to navigate.
 > For example, when the window-local tab-bar shows such tabs:
 >
 > [buffer-2] [buffer-1] [current-buffer] [buffer+1] [buffer+2]
 >
 > Then I know that I can just type `C-2 C-x left' to get to the second
 > previous buffer, or to get the second next buffer with `C-2 C-x right'.

Then we at least facultatively should show that prefix number in the
tab.  In either case you don't necessarily need window-local tabs for
that.  We could have the frame's tabbar show tabs in the order of the
frame's selected window.

martin

bug#32850: 27.0.50; window-swap-states doesn't swap window prev/next-buffers

[Juri Linkov]

>> I don't use the mouse.  Whereas window-local tab-bar showing a list
>> of prev/next-buffers allows using the keyboard to navigate.
>> For example, when the window-local tab-bar shows such tabs:
>>
>> [buffer-2] [buffer-1] [current-buffer] [buffer+1] [buffer+2]
>>
>> Then I know that I can just type `C-2 C-x left' to get to the second
>> previous buffer, or to get the second next buffer with `C-2 C-x right'.
>
> Then we at least facultatively should show that prefix number in the
> tab.

Web browsers allow switching tabs by absolute numbers, e.g. C-2 or M-2,
but they don't show these numbers in tabs.

bug#32850: 27.0.50; window-swap-states doesn't swap window prev/next-buffers

[martin rudalics]

 >> Then we at least facultatively should show that prefix number in the
 >> tab.
 >
 > Web browsers allow switching tabs by absolute numbers, e.g. C-2 or M-2,
 > but they don't show these numbers in tabs.

It would have been a pain to supply such numbers in the first place -
they would have had to be updated whenever another window or buffer
gets selected or is made current.  So forget this idea.

martin

bug#32850: 27.0.50; window-swap-states doesn't swap window prev/next-buffers

[Charles A. Roelli]

> From: Juri Linkov <juri@linkov.net>
> Date: Thu, 15 Nov 2018 23:30:48 +0200
>
> I don't use the mouse.  Whereas window-local tab-bar showing a list
> of prev/next-buffers allows using the keyboard to navigate.
> For example, when the window-local tab-bar shows such tabs:
> 
> [buffer-2] [buffer-1] [current-buffer] [buffer+1] [buffer+2]
> 
> Then I know that I can just type `C-2 C-x left' to get to the second
> previous buffer, or to get the second next buffer with `C-2 C-x right'.

C-x <left> and C-x <right> do not yet support prefix args, right?

bug#32850: 27.0.50; window-swap-states doesn't swap window prev/next-buffers

[Juri Linkov]

>> I don't use the mouse.  Whereas window-local tab-bar showing a list
>> of prev/next-buffers allows using the keyboard to navigate.
>> For example, when the window-local tab-bar shows such tabs:
>> 
>> [buffer-2] [buffer-1] [current-buffer] [buffer+1] [buffer+2]
>> 
>> Then I know that I can just type `C-2 C-x left' to get to the second
>> previous buffer, or to get the second next buffer with `C-2 C-x right'.
>
> C-x <left> and C-x <right> do not yet support prefix args, right?

Right, we need to support prefix args of C-x <left> and C-x <right>.

bug#32850: 27.0.50; window-swap-states doesn't swap window prev/next-buffers

[martin rudalics]

 >> C-x <left> and C-x <right> do not yet support prefix args, right?
 >
 > Right, we need to support prefix args of C-x <left> and C-x <right>.

Could you do that please?

Thanks, martin

bug#32850: 27.0.50; window-swap-states doesn't swap window prev/next-buffers

[Juri Linkov]

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

>>> C-x <left> and C-x <right> do not yet support prefix args, right?
>>
>> Right, we need to support prefix args of C-x <left> and C-x <right>.
>
> Could you do that please?

Like this?


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: prev-next-buffer-arg.patch --]
[-- Type: text/x-diff, Size: 2247 bytes --]

diff --git a/lisp/window.el b/lisp/window.el
index 172ee72997..1d3133c359 100644
*** a/lisp/window.el
--- b/lisp/window.el
***************
*** 4664,4690 ****
    (interactive)
    (switch-to-buffer (last-buffer)))
  
! (defun next-buffer ()
!   "In selected window switch to next buffer."
!   (interactive)
    (cond
     ((window-minibuffer-p)
      (error "Cannot switch buffers in minibuffer window"))
     ((eq (window-dedicated-p) t)
      (error "Window is strongly dedicated to its buffer"))
!    (t
!     (switch-to-next-buffer))))
! 
! (defun previous-buffer ()
!   "In selected window switch to previous buffer."
!   (interactive)
    (cond
     ((window-minibuffer-p)
      (error "Cannot switch buffers in minibuffer window"))
     ((eq (window-dedicated-p) t)
      (error "Window is strongly dedicated to its buffer"))
!    (t
!     (switch-to-prev-buffer))))
  
  (defun delete-windows-on (&optional buffer-or-name frame)
    "Delete all windows showing BUFFER-OR-NAME.
--- 4664,4698 ----
    (interactive)
    (switch-to-buffer (last-buffer)))
  
! (defun next-buffer (&optional arg)
!   "In selected window switch to next buffer.
! With a prefix argument, switch to ARGth next buffer."
!   (interactive "p")
    (cond
     ((window-minibuffer-p)
      (error "Cannot switch buffers in minibuffer window"))
     ((eq (window-dedicated-p) t)
      (error "Window is strongly dedicated to its buffer"))
!    (t (setq arg (or arg 1))
!       (if (< arg 0)
!           (previous-buffer (abs arg))
!         (while (<= 0 (setq arg (1- arg)))
!           (switch-to-next-buffer))))))
! 
! (defun previous-buffer (&optional arg)
!   "In selected window switch to previous buffer.
! With a prefix argument, switch to ARGth previous buffer."
!   (interactive "p")
    (cond
     ((window-minibuffer-p)
      (error "Cannot switch buffers in minibuffer window"))
     ((eq (window-dedicated-p) t)
      (error "Window is strongly dedicated to its buffer"))
!    (t (setq arg (or arg 1))
!       (if (< arg 0)
!           (next-buffer (abs arg))
!         (while (<= 0 (setq arg (1- arg)))
!           (switch-to-prev-buffer))))))
  
  (defun delete-windows-on (&optional buffer-or-name frame)
    "Delete all windows showing BUFFER-OR-NAME.

bug#32850: 27.0.50; window-swap-states doesn't swap window prev/next-buffers

[martin rudalics]

 >>> Right, we need to support prefix args of C-x <left> and C-x <right>.
 >>
 >> Could you do that please?
 >
 > Like this?

I doubt that

!         (while (<= 0 (setq arg (1- arg)))
!           (switch-to-next-buffer))))))

and

!         (while (<= 0 (setq arg (1- arg)))
!           (switch-to-prev-buffer))))))

will DTRT.  They pretend that all these buffers were actually shown in
the window and thus "presented" to the user.  As a consequence,
switching from a buffer B to the fourth previous buffer and
subsequently switching to the next buffer would not get us back to B
but to the third previous buffer of B at the time we switched to the
previous buffer.  I suppose you won't like that.

What we should probably do instead here is to get the fourth buffer of
what 'window-prev-buffers' returns and try to switch to that buffer
either via 'switch-to-buffer' or 'pop-to-buffer-same-window'.

martin

bug#32850: 27.0.50; window-swap-states doesn't swap window prev/next-buffers

[Juri Linkov]

>>>> Right, we need to support prefix args of C-x <left> and C-x <right>.
>>>
>>> Could you do that please?
>>
>> Like this?
>
> I doubt that
>
> !         (while (<= 0 (setq arg (1- arg)))
> !           (switch-to-next-buffer))))))
>
> and
>
> !         (while (<= 0 (setq arg (1- arg)))
> !           (switch-to-prev-buffer))))))
>
> will DTRT.  They pretend that all these buffers were actually shown in
> the window and thus "presented" to the user.  As a consequence,
> switching from a buffer B to the fourth previous buffer and
> subsequently switching to the next buffer would not get us back to B
> but to the third previous buffer of B at the time we switched to the
> previous buffer.  I suppose you won't like that.

Why not?  'C-2 C-x <left>' could be just a shortcut for
'C-x <left> C-x <left>'.

> What we should probably do instead here is to get the fourth buffer of
> what 'window-prev-buffers' returns and try to switch to that buffer
> either via 'switch-to-buffer' or 'pop-to-buffer-same-window'.

But what if the users don't want to break the chain of
previous/next buffers?  For example:

[buffer-A] [buffer-B] [buffer-C] [buffer-D] [buffer-E]
prev-buffers          current-buffer      next-buffers

After 'C-2 C-x <left>' I'd want to have the same sequence of buffers,
just changing the current buffer:

[buffer-A] [buffer-B] [buffer-C] [buffer-D] [buffer-E]
current-buffer                            next-buffers

Not what you seems to suggest that takes ARGth previous buffer
to display and breaks the order of buffers:

[buffer-B] [buffer-C] [buffer-A] [buffer-D] [buffer-E]
                      current-buffer

IOW, I use 'C-x <left>' and 'C-x <right>' to "scroll" the list
of prev/next buffers.

bug#32850: 27.0.50; window-swap-states doesn't swap window prev/next-buffers

[martin rudalics]

 > Why not?  'C-2 C-x <left>' could be just a shortcut for
 > 'C-x <left> C-x <left>'.
 >
 >> What we should probably do instead here is to get the fourth buffer of
 >> what 'window-prev-buffers' returns and try to switch to that buffer
 >> either via 'switch-to-buffer' or 'pop-to-buffer-same-window'.
 >
 > But what if the users don't want to break the chain of
 > previous/next buffers?  For example:
 >
 > [buffer-A] [buffer-B] [buffer-C] [buffer-D] [buffer-E]
 > prev-buffers          current-buffer      next-buffers
 >
 > After 'C-2 C-x <left>' I'd want to have the same sequence of buffers,
 > just changing the current buffer:
 >
 > [buffer-A] [buffer-B] [buffer-C] [buffer-D] [buffer-E]
 > current-buffer                            next-buffers
 >
 > Not what you seems to suggest that takes ARGth previous buffer
 > to display and breaks the order of buffers:
 >
 > [buffer-B] [buffer-C] [buffer-A] [buffer-D] [buffer-E]
 >                        current-buffer
 >
 > IOW, I use 'C-x <left>' and 'C-x <right>' to "scroll" the list
 > of prev/next buffers.

So what you're after is to make the visual effect of C-2 C-x <left>
the same as that of C-x b buffer-A but keep the impact on the lists of
previous buffers and next buffers distinct.  If people are OK with
that I won't insist.

But please describe the behavior in the doc-strings of
'previous-buffer' and 'next-buffer' in sufficient detail.  Also note
that the overhead induced by executing these functions then becomes
proportional to the value of the prefix argument (for each single
switch you call 'set-window-prev-buffers', 'set-window-next-buffers'
'record-window-buffer', 'unrecord-window-buffer' and run
'buffer-list-update-hook', 'window-scroll-functions' and
'window-configuration-change-hook').  Finally note that you change
'buffer-display-time' without actually displaying the buffer and the
mystic last_selected_window slot in buffer.h.

martin

bug#32850: 27.0.50; window-swap-states doesn't swap window prev/next-buffers

[Juri Linkov]

> Also note that the overhead induced by executing these functions then
> becomes proportional to the value of the prefix argument (for each
> single switch you call 'set-window-prev-buffers',
> 'set-window-next-buffers' 'record-window-buffer',
> 'unrecord-window-buffer' and run 'buffer-list-update-hook',
> 'window-scroll-functions' and 'window-configuration-change-hook').

Aren't hooks intended to run only once when the command is finished?

> Finally note that you change 'buffer-display-time' without actually
> displaying the buffer and the mystic last_selected_window slot in
> buffer.h.

Whereas 'buffer-display-time' is not very important, changing
its value looks wrong, indeed.

bug#32850: 27.0.50; window-swap-states doesn't swap window prev/next-buffers

[martin rudalics]

 >> Also note that the overhead induced by executing these functions then
 >> becomes proportional to the value of the prefix argument (for each
 >> single switch you call 'set-window-prev-buffers',
 >> 'set-window-next-buffers' 'record-window-buffer',
 >> 'unrecord-window-buffer' and run 'buffer-list-update-hook',
 >> 'window-scroll-functions' and 'window-configuration-change-hook').
 >
 > Aren't hooks intended to run only once when the command is finished?

'post-command-hook' only.  'window-size-change-functions' is currently
run once for each redisplay.  The three I cited above are run for
every set_window_buffer when the buffer changes.  Note that you are
your own client here since you want to pretend that all these buffers
were indeed displayed.

martin

bug#32850: 27.0.50; window-swap-states doesn't swap window prev/next-buffers

[Juri Linkov]

>>> Also note that the overhead induced by executing these functions then
>>> becomes proportional to the value of the prefix argument (for each
>>> single switch you call 'set-window-prev-buffers',
>>> 'set-window-next-buffers' 'record-window-buffer',
>>> 'unrecord-window-buffer' and run 'buffer-list-update-hook',
>>> 'window-scroll-functions' and 'window-configuration-change-hook').
>>
>> Aren't hooks intended to run only once when the command is finished?
>
> 'post-command-hook' only.  'window-size-change-functions' is currently
> run once for each redisplay.  The three I cited above are run for
> every set_window_buffer when the buffer changes.  Note that you are
> your own client here since you want to pretend that all these buffers
> were indeed displayed.

We could let-bind these hooks to nil in the loop, and let to call them
only for the last 'previous-buffer' call.

bug#32850: 27.0.50; window-swap-states doesn't swap window prev/next-buffers

[martin rudalics]

 > We could let-bind these hooks to nil in the loop, and let to call them
 > only for the last 'previous-buffer' call.

Maybe.  I wouldn't rely on this to work in all possible scenarios
including killed buffers and the frame's 'buffer-predicate'.

martin

bug#32850: 27.0.50; window-swap-states doesn't swap window prev/next-buffers

[martin rudalics]

 > Good idea.  Simply to add a conditional display to mode-line-format with
 >
 >    (and (not (window-in-direction 'down))
 >         (not (window-in-direction 'left)))

I forgot.  You probably mean

    (and (not (window-in-direction 'down nil nil nil nil 'nomini))
         (not (window-in-direction 'right nil nil nil nil 'nomini)))

here.

martin

bug#32850: 27.0.50; window-swap-states doesn't swap window prev/next-buffers

[Juri Linkov]

>> This patch handles killed buffers, so it works for desktop saving as well
>> (because the desktop restores the frameset only after it reads all buffers):
>
> Please install so we can see whether there are any problems (in
> particular with the desktop which I don't use).

I found a problem, it occurs very rarely:

Debugger entered--Lisp error: (wrong-type-argument stringp nil)
  get-buffer(nil)
  #f(compiled-function (entry) #<bytecode 0x22c739>)((nil nil nil))
  mapcar(#f(compiled-function (entry) #<bytecode 0x22c739>) (("*Group*" 1 139) (nil nil nil) ("*sent reply to martin rudalics*" 1 1090)))
  window--state-put-2(safe 737)
  window-state-put(...)

Do you have an idea how did (nil nil nil) find its way into the list of
prev-buffers?  (window-state-get was used with the non-nil WRITABLE arg)

bug#32850: 27.0.50; window-swap-states doesn't swap window prev/next-buffers

[martin rudalics]

 > I found a problem, it occurs very rarely:
 >
 > Debugger entered--Lisp error: (wrong-type-argument stringp nil)
 >    get-buffer(nil)
 >    #f(compiled-function (entry) #<bytecode 0x22c739>)((nil nil nil))
 >    mapcar(#f(compiled-function (entry) #<bytecode 0x22c739>) (("*Group*" 1 139) (nil nil nil) ("*sent reply to martin rudalics*" 1 1090)))
 >    window--state-put-2(safe 737)
 >    window-state-put(...)
 >
 > Do you have an idea how did (nil nil nil) find its way into the list of
 > prev-buffers?  (window-state-get was used with the non-nil WRITABLE arg)

The most simple way to find out is to define such a list in window.c
and have 'set-window-prev-buffers' holler if the prev_buffers argument
is equal to that list.

martin

bug#32850: 27.0.50; window-swap-states doesn't swap window prev/next-buffers

[Juri Linkov]

>> I found a problem, it occurs very rarely:
>>
>> Debugger entered--Lisp error: (wrong-type-argument stringp nil)
>>    get-buffer(nil)
>>    #f(compiled-function (entry) #<bytecode 0x22c739>)((nil nil nil))
>>    mapcar(#f(compiled-function (entry) #<bytecode 0x22c739>) (("*Group*" 1 139) (nil nil nil) ("*sent reply to martin rudalics*" 1 1090)))
>>    window--state-put-2(safe 737)
>>    window-state-put(...)
>>
>> Do you have an idea how did (nil nil nil) find its way into the list of
>> prev-buffers?  (window-state-get was used with the non-nil WRITABLE arg)
>
> The most simple way to find out is to define such a list in window.c
> and have 'set-window-prev-buffers' holler if the prev_buffers argument
> is equal to that list.

I can't find out how '(nil nil nil)' gets into the prev-buffers,
so to avoid the error, I propose to install this patch:

diff --git a/lisp/window.el b/lisp/window.el
index 27d7e42280..c562ed3cfa 100644
--- a/lisp/window.el
+++ b/lisp/window.el
@@ -5854,14 +5854,15 @@ window--state-put-2
                   (set-window-next-buffers
                    window
                    (delq nil (mapcar (lambda (buffer)
-                                       (setq buffer (get-buffer buffer))
+                                       (setq buffer (and buffer (get-buffer buffer)))
                                        (when (buffer-live-p buffer) buffer))
                                      next-buffers))))
                 (when prev-buffers
                   (set-window-prev-buffers
                    window
                    (delq nil (mapcar (lambda (entry)
-                                       (let ((buffer (get-buffer (nth 0 entry)))
+                                       (let ((buffer (and (nth 0 entry)
+                                                          (get-buffer (nth 0 entry))))
                                              (m1 (nth 1 entry))
                                              (m2 (nth 2 entry)))
                                          (when (buffer-live-p buffer)

bug#32850: 27.0.50; window-swap-states doesn't swap window prev/next-buffers

[martin rudalics]

 >> The most simple way to find out is to define such a list in window.c
 >> and have 'set-window-prev-buffers' holler if the prev_buffers argument
 >> is equal to that list.
 >
 > I can't find out how '(nil nil nil)' gets into the prev-buffers,

Did you try what I proposed?  If that doesn't catch it, I have no
idea.  IIUC mark_discard_killed_buffers just removes entries from that
list, it cannot introduce any nil valued elements.

 > so to avoid the error, I propose to install this patch:

Well this will just cache the underlying problem.  But I have no good
idea how to investigate this issue any furher, so install if you like.

martin

bug#32850: 27.0.50; window-swap-states doesn't swap window prev/next-buffers

[Juri Linkov]

>>> The most simple way to find out is to define such a list in window.c
>>> and have 'set-window-prev-buffers' holler if the prev_buffers argument
>>> is equal to that list.
>>
>> I can't find out how '(nil nil nil)' gets into the prev-buffers,
>
> Did you try what I proposed?  If that doesn't catch it, I have no
> idea.  IIUC mark_discard_killed_buffers just removes entries from that
> list, it cannot introduce any nil valued elements.
>
>> so to avoid the error, I propose to install this patch:
>
> Well this will just cache the underlying problem.  But I have no good
> idea how to investigate this issue any furher, so install if you like.

I tried this advice:

(advice-add 'set-window-prev-buffers :before
	    (lambda (window prev-buffers)
	      (when (member '(nil nil nil) prev-buffers)
	        (let (frames)
		  (mapbacktrace (lambda (evald fun args flags)
				  (push fun frames)))
		  (display-warning 'prev-buffers
				   (format "%S %S" prev-buffers (nreverse frames))
				   :warning))))
	    '((name . debug)))

but it still doesn't catch (nil nil nil) added to prev-buffers
and here is a new backtrace:

Debugger entered--Lisp error: (wrong-type-argument stringp nil)
  get-buffer(nil)
  #f(compiled-function (entry) #<bytecode 0x22c73d>)((nil nil nil))
  mapcar(#f(compiled-function (entry) #<bytecode 0x22c73d>) (("*Help*" 1 112) (nil nil nil)))
  window--state-put-2(safe 737)
  window-state-put((... (buffer "isearch.el" ...) (prev-buffers ("*Help*" 1 112) (nil nil nil))) #<window 3 on isearch.el> safe)
  frameset--restore-frame(... t)
  frameset-restore([frameset ...] :reuse-frames t :cleanup-frames t :force-display t :force-onscreen t)
  desktop-restore-frameset()
  desktop-read()
  #f(compiled-function () #<bytecode 0x13ecc2d>)()
  run-hooks(after-init-hook delayed-warnings-hook)
  command-line()
  normal-top-level()

bug#32850: 27.0.50; window-swap-states doesn't swap window prev/next-buffers

[Juri Linkov]

>>> The most simple way to find out is to define such a list in window.c
>>> and have 'set-window-prev-buffers' holler if the prev_buffers argument
>>> is equal to that list.
>>
>> I can't find out how '(nil nil nil)' gets into the prev-buffers,
>
> Did you try what I proposed?  If that doesn't catch it, I have no
> idea.  IIUC mark_discard_killed_buffers just removes entries from that
> list, it cannot introduce any nil valued elements.
>
>> so to avoid the error, I propose to install this patch:
>
> Well this will just cache the underlying problem.  But I have no good
> idea how to investigate this issue any furher, so install if you like.

Now I found a reproducible test case:

1. create and remember a window-configuration

2. switch to a different window-configuration

3. kill the buffer that was in prev-buffers of the
   first window-configuration

4. restore the first window-configuration

5. look into its prev-buffers, it contains:

   (#<killed buffer> #<marker in no buffer> #<marker in no buffer>)

6. (window-state-get nil t) on the window with such prev-buffers
    produces:

    (nil nil nil)

I believe this patch should fix the problem:

diff --git a/lisp/window.el b/lisp/window.el
index 27d7e42280..ad8164052f 100644
--- a/lisp/window.el
+++ b/lisp/window.el
@@ -5552,9 +5552,14 @@ window--state-get-1
 	 (buffer (window-buffer window))
 	 (selected (eq window (selected-window)))
 	 (next-buffers (when (window-live-p window)
-			 (window-next-buffers window)))
+	                 (delq nil (mapcar (lambda (buffer)
+                                             (and (buffer-live-p buffer) buffer))
+                                           (window-next-buffers window)))))
 	 (prev-buffers (when (window-live-p window)
-			 (window-prev-buffers window)))
+	                 (delq nil (mapcar (lambda (entry)
+                                             (and (buffer-live-p (nth 0 entry))
+                                                  entry))
+                                           (window-prev-buffers window)))))
 	 (head
 	  `(,type
             ,@(unless (window-next-sibling window) `((last . t)))

bug#32850: 27.0.50; window-swap-states doesn't swap window prev/next-buffers

[martin rudalics]

 > Now I found a reproducible test case:
 >
 > 1. create and remember a window-configuration
 >
 > 2. switch to a different window-configuration
 >
 > 3. kill the buffer that was in prev-buffers of the
 >     first window-configuration
 >
 > 4. restore the first window-configuration
 >
 > 5. look into its prev-buffers, it contains:
 >
 >     (#<killed buffer> #<marker in no buffer> #<marker in no buffer>)

Hmmm... so even live windows can make troubles as soon as they are
stored in a configuration.  You have probably seen the tribulations of
'switch-to-prev-buffer' and 'switch-to-next-buffer' when they
encounter a killed buffer in one of these lists.

 > 6. (window-state-get nil t) on the window with such prev-buffers
 >      produces:
 >
 >      (nil nil nil)
 >
 > I believe this patch should fix the problem:
 >
 > diff --git a/lisp/window.el b/lisp/window.el
 > index 27d7e42280..ad8164052f 100644
 > --- a/lisp/window.el
 > +++ b/lisp/window.el
 > @@ -5552,9 +5552,14 @@ window--state-get-1
 >   	 (buffer (window-buffer window))
 >   	 (selected (eq window (selected-window)))
 >   	 (next-buffers (when (window-live-p window)
 > -			 (window-next-buffers window)))
 > +	                 (delq nil (mapcar (lambda (buffer)
 > +                                             (and (buffer-live-p buffer) buffer))
 > +                                           (window-next-buffers window)))))
 >   	 (prev-buffers (when (window-live-p window)
 > -			 (window-prev-buffers window)))
 > +	                 (delq nil (mapcar (lambda (entry)
 > +                                             (and (buffer-live-p (nth 0 entry))
 > +                                                  entry))
 > +                                           (window-prev-buffers window)))))
 >   	 (head
 >   	  `(,type
 >               ,@(unless (window-next-sibling window) `((last . t)))

OK.  Wouldn't 'window-state-put' run into a similar problem when a
buffer in some prev or next list gets killed in between getting and
putting?

martin

bug#32850: 27.0.50; window-swap-states doesn't swap window prev/next-buffers

[Juri Linkov]

>> @@ -5552,9 +5552,14 @@ window--state-get-1
>>   	 (buffer (window-buffer window))
>>   	 (selected (eq window (selected-window)))
>>   	 (next-buffers (when (window-live-p window)
>> -			 (window-next-buffers window)))
>> +	                 (delq nil (mapcar (lambda (buffer)
>> +                                             (and (buffer-live-p buffer) buffer))
>> +                                           (window-next-buffers window)))))
>>   	 (prev-buffers (when (window-live-p window)
>> -			 (window-prev-buffers window)))
>> +	                 (delq nil (mapcar (lambda (entry)
>> +                                             (and (buffer-live-p (nth 0 entry))
>> +                                                  entry))
>> +                                           (window-prev-buffers window)))))
>>   	 (head
>>   	  `(,type
>>               ,@(unless (window-next-sibling window) `((last . t)))
>
> OK.  Wouldn't 'window-state-put' run into a similar problem when a
> buffer in some prev or next list gets killed in between getting and
> putting?

'window-state-put' already checks for 'buffer-live-p', and checking for nil
is not needed in 'window-state-put' after the patch above will protect
against nil getting into the window state.

bug#32850: 27.0.50; window-swap-states doesn't swap window prev/next-buffers

[martin rudalics]

 > 'window-state-put' already checks for 'buffer-live-p', and checking for nil
 > is not needed in 'window-state-put' after the patch above will protect
 > against nil getting into the window state.

OK.  Install then.

Thanks, martin