unofficial mirror of bug-gnu-emacs@gnu.org 
 help / color / mirror / code / Atom feed
* bug#69093: window-state-put doesn't update current buffer
@ 2024-02-13  7:39 Juri Linkov
  2024-02-14  9:12 ` martin rudalics via Bug reports for GNU Emacs, the Swiss army knife of text editors
  0 siblings, 1 reply; 20+ messages in thread
From: Juri Linkov @ 2024-02-13  7:39 UTC (permalink / raw)
  To: 69093; +Cc: martin rudalics

Martin, could you help to understand what is missing in
'window-state-put' that it doesn't set the current buffer
correctly like 'set-window-configuration' does.

For example, here the message says that the current-buffer
and point still have old values coming from the old buffer
that was current before calling 'window-state-put':

(let (ws)
  (info)
  (setq ws (window-state-get nil 'writable))
  (quit-window)
  (window-state-put ws nil 'safe)
  (message "! %S %S %S" (point) (current-buffer) (window-buffer)))

However, 'set-window-configuration' works correctly
and sets the restored buffer as current:

(let (wc)
  (info)
  (setq wc (current-window-configuration))
  (quit-window)
  (set-window-configuration wc nil t)
  (message "! %S %S %S" (point) (current-buffer) (window-buffer)))





^ permalink raw reply	[flat|nested] 20+ messages in thread

* bug#69093: window-state-put doesn't update current buffer
  2024-02-13  7:39 bug#69093: window-state-put doesn't update current buffer Juri Linkov
@ 2024-02-14  9:12 ` martin rudalics via Bug reports for GNU Emacs, the Swiss army knife of text editors
  2024-02-15  7:29   ` Juri Linkov
  0 siblings, 1 reply; 20+ messages in thread
From: martin rudalics via Bug reports for GNU Emacs, the Swiss army knife of text editors @ 2024-02-14  9:12 UTC (permalink / raw)
  To: Juri Linkov, 69093

 > Martin, could you help to understand what is missing in
 > 'window-state-put' that it doesn't set the current buffer
 > correctly like 'set-window-configuration' does.

The "current buffer" is not part of the state of a window.  It is part
of a more global state.  Have a look at frameset.el which does at some
time "Restore selected frame, buffer and point."

As for what 'set-window-configuration' additionally does, have a look at
'current-window-configuration' where you can see that besides

   XSETBUFFER (data->f_current_buffer, current_buffer);

it also saves the selected frame, scroll and selected windows of that
frame's minibuffer or the frame that should receive input focus.

martin





^ permalink raw reply	[flat|nested] 20+ messages in thread

* bug#69093: window-state-put doesn't update current buffer
  2024-02-14  9:12 ` martin rudalics via Bug reports for GNU Emacs, the Swiss army knife of text editors
@ 2024-02-15  7:29   ` Juri Linkov
  2024-02-16  9:39     ` martin rudalics via Bug reports for GNU Emacs, the Swiss army knife of text editors
  0 siblings, 1 reply; 20+ messages in thread
From: Juri Linkov @ 2024-02-15  7:29 UTC (permalink / raw)
  To: martin rudalics; +Cc: 69093

>> Martin, could you help to understand what is missing in
>> 'window-state-put' that it doesn't set the current buffer
>> correctly like 'set-window-configuration' does.
>
> The "current buffer" is not part of the state of a window.  It is part
> of a more global state.  Have a look at frameset.el which does at some
> time "Restore selected frame, buffer and point."
>
> As for what 'set-window-configuration' additionally does, have a look at
> 'current-window-configuration' where you can see that besides
>
>   XSETBUFFER (data->f_current_buffer, current_buffer);
>
> it also saves the selected frame, scroll and selected windows of that
> frame's minibuffer or the frame that should receive input focus.

Thanks for explanations.  I see this line in 'set-window-configuration':

      Fset_buffer (new_current_buffer);

Do you think this is the right fix?

diff --git a/lisp/window.el b/lisp/window.el
index 6df20353b5e..34e6c5d4a4f 100644
--- a/lisp/window.el
+++ b/lisp/window.el
@@ -6498,7 +6503,8 @@ window-state-put
 	  (when (and (window-valid-p window)
                      (eq (window-deletable-p window) t))
 	    (delete-window window))))
-      (window--check frame))))
+      (window--check frame)
+      (set-buffer (window-buffer)))))
 
 (defun window-state-buffers (state)
   "Return all buffers saved to the given window state STATE."





^ permalink raw reply related	[flat|nested] 20+ messages in thread

* bug#69093: window-state-put doesn't update current buffer
  2024-02-15  7:29   ` Juri Linkov
@ 2024-02-16  9:39     ` martin rudalics via Bug reports for GNU Emacs, the Swiss army knife of text editors
  2024-02-16 11:46       ` Eli Zaretskii
  2024-02-18  7:43       ` Juri Linkov
  0 siblings, 2 replies; 20+ messages in thread
From: martin rudalics via Bug reports for GNU Emacs, the Swiss army knife of text editors @ 2024-02-16  9:39 UTC (permalink / raw)
  To: Juri Linkov; +Cc: 69093

 > Thanks for explanations.  I see this line in 'set-window-configuration':
 >
 >        Fset_buffer (new_current_buffer);
 >
 > Do you think this is the right fix?

No.  I think the right fix would be to remove the above line from
'set-window-configuration'.  We can't do that because some applications
might depend on the current behavior.  But I am quite confident that
nobody fully understands 'set-window-configuration' anyway and can
predict what it does when selected frame, current buffer and the buffer
and frame stored in the CONFIGURATION argument mismatch.

Or could you tell beforehand which buffer will be current after

(let ((configuration (current-window-configuration)))
   (pop-to-buffer "*Messages*" '((display-buffer-pop-up-frame)))
   (set-window-configuration configuration)
   (current-buffer))

I think that the behavior of

(let ((frame (selected-frame))
       (state (window-state-get)))
   (pop-to-buffer "*Messages*" '((display-buffer-pop-up-frame)))
   (window-state-put state (frame-root-window frame))
   (current-buffer))

is much more consistent in this regard.

I'd say that any code run in a state where the buffer of the selected
window and the current buffer are not the same - regardless of whether
this happens when a state/configuration is saved or restored - should
simply report an error.  But that ship has sailed long ago.

martin





^ permalink raw reply	[flat|nested] 20+ messages in thread

* bug#69093: window-state-put doesn't update current buffer
  2024-02-16  9:39     ` martin rudalics via Bug reports for GNU Emacs, the Swiss army knife of text editors
@ 2024-02-16 11:46       ` Eli Zaretskii
  2024-02-18  7:43       ` Juri Linkov
  1 sibling, 0 replies; 20+ messages in thread
From: Eli Zaretskii @ 2024-02-16 11:46 UTC (permalink / raw)
  To: martin rudalics; +Cc: 69093, juri

> Cc: 69093@debbugs.gnu.org
> Date: Fri, 16 Feb 2024 10:39:36 +0100
> From:  martin rudalics via "Bug reports for GNU Emacs,
>  the Swiss army knife of text editors" <bug-gnu-emacs@gnu.org>
> 
>  > Thanks for explanations.  I see this line in 'set-window-configuration':
>  >
>  >        Fset_buffer (new_current_buffer);
>  >
>  > Do you think this is the right fix?
> 
> No.  I think the right fix would be to remove the above line from
> 'set-window-configuration'.  We can't do that because some applications
> might depend on the current behavior.  But I am quite confident that
> nobody fully understands 'set-window-configuration' anyway and can
> predict what it does when selected frame, current buffer and the buffer
> and frame stored in the CONFIGURATION argument mismatch.

A more worrisome aspect of such a change would be the gazillion uses
of save-window-excursion.

> I'd say that any code run in a state where the buffer of the selected
> window and the current buffer are not the same - regardless of whether
> this happens when a state/configuration is saved or restored - should
> simply report an error.

Some do, others don't.  Still others silently correct the mismatch.





^ permalink raw reply	[flat|nested] 20+ messages in thread

* bug#69093: window-state-put doesn't update current buffer
  2024-02-16  9:39     ` martin rudalics via Bug reports for GNU Emacs, the Swiss army knife of text editors
  2024-02-16 11:46       ` Eli Zaretskii
@ 2024-02-18  7:43       ` Juri Linkov
  2024-02-19  9:43         ` martin rudalics via Bug reports for GNU Emacs, the Swiss army knife of text editors
  1 sibling, 1 reply; 20+ messages in thread
From: Juri Linkov @ 2024-02-18  7:43 UTC (permalink / raw)
  To: martin rudalics; +Cc: 69093

> Or could you tell beforehand which buffer will be current after
>
> (let ((configuration (current-window-configuration)))
>   (pop-to-buffer "*Messages*" '((display-buffer-pop-up-frame)))
>   (set-window-configuration configuration)
>   (current-buffer))

Does this ambiguity exist only for multi-frame setups where
a system window manager decides what frame to select afterwards?





^ permalink raw reply	[flat|nested] 20+ messages in thread

* bug#69093: window-state-put doesn't update current buffer
  2024-02-18  7:43       ` Juri Linkov
@ 2024-02-19  9:43         ` martin rudalics via Bug reports for GNU Emacs, the Swiss army knife of text editors
  2024-02-19 17:36           ` Drew Adams
  2024-02-20  7:40           ` Juri Linkov
  0 siblings, 2 replies; 20+ messages in thread
From: martin rudalics via Bug reports for GNU Emacs, the Swiss army knife of text editors @ 2024-02-19  9:43 UTC (permalink / raw)
  To: Juri Linkov; +Cc: 69093

 >> Or could you tell beforehand which buffer will be current after
 >>
 >> (let ((configuration (current-window-configuration)))
 >>    (pop-to-buffer "*Messages*" '((display-buffer-pop-up-frame)))
 >>    (set-window-configuration configuration)
 >>    (current-buffer))
 >
 > Does this ambiguity exist only for multi-frame setups where
 > a system window manager decides what frame to select afterwards?

The system window manager here does only what Emacs tells it to do.  But
note that multi-frame setups got more and more broken since Emacs 26 and
are maybe not used frequently nowadays.  So you probably needn't give
them much attention in the first place.

martin





^ permalink raw reply	[flat|nested] 20+ messages in thread

* bug#69093: window-state-put doesn't update current buffer
  2024-02-19  9:43         ` martin rudalics via Bug reports for GNU Emacs, the Swiss army knife of text editors
@ 2024-02-19 17:36           ` Drew Adams
  2024-02-20  7:40           ` Juri Linkov
  1 sibling, 0 replies; 20+ messages in thread
From: Drew Adams @ 2024-02-19 17:36 UTC (permalink / raw)
  To: martin rudalics, Juri Linkov; +Cc: 69093@debbugs.gnu.org

> note that multi-frame setups got more and more
> broken since Emacs 26

Yes.  Unfortunately the passive voice is needed
here because much has changed without our being
able to identify and say who/what changed what.

> and are maybe not used frequently nowadays.

They _never_ were frequently used.  They were
standard in Epoch, but never integrated into
GNU Emacs OOTB.  GNU Emacs users of them have
always been outliers.  Dunno whether Stefan M
still uses a standalone minibuffer.  If not,
I might be the only user who does.

> So you probably needn't give them much
> attention in the first place.

That's the problem in the first place, and IMO
the reason they've never been used frequently.
Emacs developers don't, themselves, use them,
and so don't test changes against them,...
Vicious circle.  Too bad.

(No, I don't have the time anymore to try to
track down what might have broken what & when.
Just saying that this is unfortunate.  Things
might have been different if GNU Emacs had,
many moon ago, provided a standalone minibuffer
OOTB, a la Epoch.)

^ permalink raw reply	[flat|nested] 20+ messages in thread

* bug#69093: window-state-put doesn't update current buffer
  2024-02-19  9:43         ` martin rudalics via Bug reports for GNU Emacs, the Swiss army knife of text editors
  2024-02-19 17:36           ` Drew Adams
@ 2024-02-20  7:40           ` Juri Linkov
  2024-02-21  9:04             ` martin rudalics via Bug reports for GNU Emacs, the Swiss army knife of text editors
  1 sibling, 1 reply; 20+ messages in thread
From: Juri Linkov @ 2024-02-20  7:40 UTC (permalink / raw)
  To: martin rudalics; +Cc: 69093

>>> Or could you tell beforehand which buffer will be current after
>>>
>>> (let ((configuration (current-window-configuration)))
>>>    (pop-to-buffer "*Messages*" '((display-buffer-pop-up-frame)))
>>>    (set-window-configuration configuration)
>>>    (current-buffer))
>>
>> Does this ambiguity exist only for multi-frame setups where
>> a system window manager decides what frame to select afterwards?
>
> The system window manager here does only what Emacs tells it to do.  But
> note that multi-frame setups got more and more broken since Emacs 26 and
> are maybe not used frequently nowadays.  So you probably needn't give
> them much attention in the first place.

Ok, since it's impossible to fix 'window-state-put',
let's narrow the scope just to the single-frame case
in the caller 'tab-bar-select-tab':

diff --git a/lisp/tab-bar.el b/lisp/tab-bar.el
index d653f339fea..efca893fc16 100644
--- a/lisp/tab-bar.el
+++ b/lisp/tab-bar.el
@@ -1477,7 +1489,8 @@ tab-bar-select-tab
           ;; `window-state-put' fails when called in the minibuffer
           (when (window-minibuffer-p)
             (select-window (get-mru-window)))
-          (window-state-put ws nil 'safe)))
+          (window-state-put ws nil 'safe)
+          (set-buffer (window-buffer))))
 
         ;; Select the minibuffer when it was active before switching tabs
         (when (and minibuffer-was-active (active-minibuffer-window))






^ permalink raw reply related	[flat|nested] 20+ messages in thread

* bug#69093: window-state-put doesn't update current buffer
  2024-02-20  7:40           ` Juri Linkov
@ 2024-02-21  9:04             ` martin rudalics via Bug reports for GNU Emacs, the Swiss army knife of text editors
  2024-02-21 17:27               ` Juri Linkov
  0 siblings, 1 reply; 20+ messages in thread
From: martin rudalics via Bug reports for GNU Emacs, the Swiss army knife of text editors @ 2024-02-21  9:04 UTC (permalink / raw)
  To: Juri Linkov; +Cc: 69093

I'd say that saving a window configuration where the current buffer is
_not_ shown in the selected window is never right.  So can you give me a
practical example where you think that this

 > +          (set-buffer (window-buffer))))

is useful or needed?

martin





^ permalink raw reply	[flat|nested] 20+ messages in thread

* bug#69093: window-state-put doesn't update current buffer
  2024-02-21  9:04             ` martin rudalics via Bug reports for GNU Emacs, the Swiss army knife of text editors
@ 2024-02-21 17:27               ` Juri Linkov
  2024-02-22  8:58                 ` martin rudalics via Bug reports for GNU Emacs, the Swiss army knife of text editors
  0 siblings, 1 reply; 20+ messages in thread
From: Juri Linkov @ 2024-02-21 17:27 UTC (permalink / raw)
  To: martin rudalics; +Cc: 69093

> I'd say that saving a window configuration where the current buffer is
> _not_ shown in the selected window is never right.  So can you give me a
> practical example where you think that this
>
>> +          (set-buffer (window-buffer))))
>
> is useful or needed?

This fix is needed for this case:

(defun pulse-momentary-highlight-one-line (&optional point face)
  (save-excursion
    (goto-char (or point (point)))
    (let ((start (progn (vertical-motion 0) (point)))
          (end (progn (vertical-motion 1) (point))))
      (pulse-momentary-highlight-region start end face))))

It expects that 'point' should be in the current buffer
that is displayed in the selected window.

But this hook fails:

(add-hook 'tab-bar-tab-post-select-functions
          (lambda (_from-tab _to-tab)
            (pulse-momentary-highlight-one-line)))

because this new hook is called in the patch below
after finishing 'window-state-put' that doesn't
set the current buffer to the window's buffer.
So after 'window-state-put' finishes,
the current buffer stays in some previous buffer.

diff --git a/lisp/tab-bar.el b/lisp/tab-bar.el
index 3e1d8278b04..50679f54474 100644
--- a/lisp/tab-bar.el
+++ b/lisp/tab-bar.el
@@ -1499,7 +1565,10 @@ tab-bar-select-tab
               (tab-bar--current-tab-make (nth to-index tabs)))
 
         (unless tab-bar-mode
-          (message "Selected tab '%s'" (alist-get 'name to-tab))))
+          (message "Selected tab '%s'" (alist-get 'name to-tab)))
+
+        (run-hook-with-args 'tab-bar-tab-post-select-functions
+                            from-tab to-tab))
 
       (force-mode-line-update))))
 





^ permalink raw reply related	[flat|nested] 20+ messages in thread

* bug#69093: window-state-put doesn't update current buffer
  2024-02-21 17:27               ` Juri Linkov
@ 2024-02-22  8:58                 ` martin rudalics via Bug reports for GNU Emacs, the Swiss army knife of text editors
  2024-02-22 17:23                   ` Juri Linkov
  0 siblings, 1 reply; 20+ messages in thread
From: martin rudalics via Bug reports for GNU Emacs, the Swiss army knife of text editors @ 2024-02-22  8:58 UTC (permalink / raw)
  To: Juri Linkov; +Cc: 69093

 > This fix is needed for this case:
 >
 > (defun pulse-momentary-highlight-one-line (&optional point face)
 >    (save-excursion
 >      (goto-char (or point (point)))
 >      (let ((start (progn (vertical-motion 0) (point)))
 >            (end (progn (vertical-motion 1) (point))))
 >        (pulse-momentary-highlight-region start end face))))
 >
 > It expects that 'point' should be in the current buffer
 > that is displayed in the selected window.

This function will probably not DTRT when the same buffer is displayed
in two windows with different values of point.  It should use an overlay
with a 'window' property.

 > But this hook fails:
 >
 > (add-hook 'tab-bar-tab-post-select-functions
 >            (lambda (_from-tab _to-tab)
 >              (pulse-momentary-highlight-one-line)))
 >
 > because this new hook is called in the patch below
 > after finishing 'window-state-put' that doesn't
 > set the current buffer to the window's buffer.
 > So after 'window-state-put' finishes,
 > the current buffer stays in some previous buffer.

If by "fails" you mean that 'window-state-put' does not select the
window selected at the time the corresponding 'window-state-get' was
run, then you should fix this in the tab bar code by recording the
frame's selected window together with the state and, depending on
whether the frame you put the state into is selected or not, either set
that frame's selected window or select that window.  The latter case
should then make that window's buffer current.

If by "fails" you mean that something in ‘window-state-put’ makes the
selected window not show the current buffer, we have to dig further.

But in neither case use 'set-buffer' to fix a window/buffer relationship
that has been screwed up elsewhere.

martin

^ permalink raw reply	[flat|nested] 20+ messages in thread

* bug#69093: window-state-put doesn't update current buffer
  2024-02-22  8:58                 ` martin rudalics via Bug reports for GNU Emacs, the Swiss army knife of text editors
@ 2024-02-22 17:23                   ` Juri Linkov
  2024-02-23  8:48                     ` martin rudalics via Bug reports for GNU Emacs, the Swiss army knife of text editors
  0 siblings, 1 reply; 20+ messages in thread
From: Juri Linkov @ 2024-02-22 17:23 UTC (permalink / raw)
  To: martin rudalics; +Cc: 69093

>> (defun pulse-momentary-highlight-one-line (&optional point face)
>>    (save-excursion
>>      (goto-char (or point (point)))
>>      (let ((start (progn (vertical-motion 0) (point)))
>>            (end (progn (vertical-motion 1) (point))))
>>        (pulse-momentary-highlight-region start end face))))
>>
>> It expects that 'point' should be in the current buffer
>> that is displayed in the selected window.
>
> This function will probably not DTRT when the same buffer is displayed
> in two windows with different values of point.  It should use an overlay
> with a 'window' property.

Agreed, a 'window' property would be nice.

> If by "fails" you mean that 'window-state-put' does not select the
> window selected at the time the corresponding 'window-state-get' was
> run, then you should fix this in the tab bar code by recording the
> frame's selected window together with the state and, depending on
> whether the frame you put the state into is selected or not, either set
> that frame's selected window or select that window.  The latter case
> should then make that window's buffer current.

The window state already has information about the selected window:

  (selected . t)

> If by "fails" you mean that something in ‘window-state-put’ makes the
> selected window not show the current buffer, we have to dig further.

'window-state-put' fails to select the previously selected
window's buffer with the property (selected . t).





^ permalink raw reply	[flat|nested] 20+ messages in thread

* bug#69093: window-state-put doesn't update current buffer
  2024-02-22 17:23                   ` Juri Linkov
@ 2024-02-23  8:48                     ` martin rudalics via Bug reports for GNU Emacs, the Swiss army knife of text editors
  2024-02-24 17:32                       ` Juri Linkov
  0 siblings, 1 reply; 20+ messages in thread
From: martin rudalics via Bug reports for GNU Emacs, the Swiss army knife of text editors @ 2024-02-23  8:48 UTC (permalink / raw)
  To: Juri Linkov; +Cc: 69093

 >> If by "fails" you mean that something in ‘window-state-put’ makes the
 >> selected window not show the current buffer, we have to dig further.
 >
 > The window state already has information about the selected window:
 >
 >    (selected . t)

I forgot.  If the selected window is part of the saved state, this is
set.

 > 'window-state-put' fails to select the previously selected
 > window's buffer with the property (selected . t).

It should have selected it here

		;; Select window if it's the selected one.
		(when (cdr (assq 'selected state))
		  (select-window window))

Please first check whether this 'select-window' call is executed at all
in your scenario.  If it is, please find out which window apparently
gets selected instead afterwards and try to find out why.  If there's no
clue, you would have to find out who undoes that selection first by
putting a breakpoint into Fselect_window and, if that fails, by putting
a breakpoint into select_window (which can be a pain).

martin

^ permalink raw reply	[flat|nested] 20+ messages in thread

* bug#69093: window-state-put doesn't update current buffer
  2024-02-23  8:48                     ` martin rudalics via Bug reports for GNU Emacs, the Swiss army knife of text editors
@ 2024-02-24 17:32                       ` Juri Linkov
  2024-02-25  9:17                         ` martin rudalics via Bug reports for GNU Emacs, the Swiss army knife of text editors
  0 siblings, 1 reply; 20+ messages in thread
From: Juri Linkov @ 2024-02-24 17:32 UTC (permalink / raw)
  To: martin rudalics; +Cc: 69093

>> 'window-state-put' fails to select the previously selected
>> window's buffer with the property (selected . t).
>
> It should have selected it here
>
> 		;; Select window if it's the selected one.
> 		(when (cdr (assq 'selected state))
> 		  (select-window window))
>
> Please first check whether this 'select-window' call is executed at all
> in your scenario.  If it is, please find out which window apparently
> gets selected instead afterwards and try to find out why.

The right window is selected, and its buffer becomes current.

> If there's no clue, you would have to find out who undoes that
> selection first by putting a breakpoint into Fselect_window and, if
> that fails, by putting a breakpoint into select_window (which can be
> a pain).

The problem is that afterwards the same function undoes the setting of
current buffer.  In window--state-put-2, select-window is inside
with-current-buffer that undoes the current buffer selection:

	      (with-current-buffer buffer
                ...
		;; Select window if it's the selected one.
		(when (cdr (assq 'selected state))
                  (select-window window))

Also in window-state-put, with-temp-buffer undoes the current buffer:

      (with-temp-buffer
	(set-window-buffer window (current-buffer))
	(window--state-put-1 state window nil totals pixelwise)
	(window--state-put-2 ignore pixelwise))





^ permalink raw reply	[flat|nested] 20+ messages in thread

* bug#69093: window-state-put doesn't update current buffer
  2024-02-24 17:32                       ` Juri Linkov
@ 2024-02-25  9:17                         ` martin rudalics via Bug reports for GNU Emacs, the Swiss army knife of text editors
  2024-02-25 18:19                           ` Juri Linkov
  0 siblings, 1 reply; 20+ messages in thread
From: martin rudalics via Bug reports for GNU Emacs, the Swiss army knife of text editors @ 2024-02-25  9:17 UTC (permalink / raw)
  To: Juri Linkov; +Cc: 69093

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

 > The problem is that afterwards the same function undoes the setting of
 > current buffer.  In window--state-put-2, select-window is inside
 > with-current-buffer that undoes the current buffer selection:
 >
 > 	      (with-current-buffer buffer
 >                  ...
 > 		;; Select window if it's the selected one.
 > 		(when (cdr (assq 'selected state))
 >                    (select-window window))
 >
 > Also in window-state-put, with-temp-buffer undoes the current buffer:
 >
 >        (with-temp-buffer
 > 	(set-window-buffer window (current-buffer))
 > 	(window--state-put-1 state window nil totals pixelwise)
 > 	(window--state-put-2 ignore pixelwise))

Silly me.  So far this apparently never caused any problems because
command_loop_1 makes the buffer of the selected window current.  But it
will affect any code running after calling 'window-state-put' up to and
including the next 'post-command-hook'.

The attached patch (including the former changes) should fix it.  I left
the old 'select-window' call in just for the case that its effect is
used elsewhere in a function called by 'window--state-put-2' later.

Many thanks for the analysis, martin

[-- Attachment #2: keep-windows.diff --]
[-- Type: text/x-patch, Size: 12507 bytes --]

diff --git a/lisp/window.el b/lisp/window.el
index e100f25526b..8e966b594f8 100644
--- a/lisp/window.el
+++ b/lisp/window.el
@@ -6174,6 +6174,12 @@ window-state-put-list
 (defvar window-state-put-stale-windows nil
   "Helper variable for `window-state-put'.")
 
+(defvar window-state-put-kept-windows nil
+  "Helper variable for `window-state-put'.")
+
+(defvar window-state-put-selected-window nil
+  "Helper variable for `window-state-put'.")
+
 (defun window--state-put-1 (state &optional window ignore totals pixelwise)
   "Helper function for `window-state-put'."
   (let ((type (car state)))
@@ -6278,9 +6284,10 @@ window--state-put-2
 	  (set-window-parameter window (car parameter) (cdr parameter))))
       ;; Process buffer related state.
       (when state
-	(let ((buffer (get-buffer (car state)))
-	      (state (cdr state)))
-	  (if buffer
+	(let* ((old-buffer-or-name (car state))
+	       (buffer (get-buffer old-buffer-or-name))
+	       (state (cdr state)))
+	  (if (buffer-live-p buffer)
 	      (with-current-buffer buffer
 		(set-window-buffer window buffer)
 		(set-window-hscroll window (cdr (assq 'hscroll state)))
@@ -6348,7 +6355,18 @@ 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))
+		  ;; This used to call 'select-window' which, however,
+		  ;; can be partially undone because the current buffer
+		  ;; may subsequently change twice: When leaving the
+		  ;; present 'with-current-buffer' and when leaving the
+		  ;; containing 'with-temp-buffer' form (Bug#69093).
+		  ;; 'window-state-put-selected-window' should now work
+		  ;; around that bug but we leave this 'select-window'
+		  ;; in since some code run before the part that fixed
+		  ;; it might still refer to this window as the selected
+		  ;; one.
+		  (select-window window)
+		  (setq window-state-put-selected-window window))
                 (set-window-next-buffers
                  window
                  (delq nil (mapcar (lambda (buffer)
@@ -6375,7 +6393,20 @@ window--state-put-2
 	    ;; save the window with the intention of deleting it later
 	    ;; if possible.
 	    (switch-to-prev-buffer window)
-	    (push window window-state-put-stale-windows)))))))
+	    (if window-kept-windows-functions
+		(let* ((start (cdr (assq 'start state)))
+		       ;; Handle both - marker positions from writable
+		       ;; states and markers from non-writable states.
+		       (start-pos (if (markerp start)
+				      (marker-last-position start)
+				    start))
+		       (point (cdr (assq 'point state)))
+		       (point-pos (if (markerp point)
+				      (marker-last-position point)
+				    point)))
+		  (push (list window old-buffer-or-name start-pos point-pos)
+			window-state-put-kept-windows))
+	      (push window window-state-put-stale-windows))))))))
 
 (defun window-state-put (state &optional window ignore)
   "Put window state STATE into WINDOW.
@@ -6388,8 +6419,20 @@ window-state-put
 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'."
+`window-safe-min-width'.
+
+If the abnormal hook `window-kept-windows-functions' is non-nil,
+never delete any windows saved by STATE whose buffers were
+deleted since STATE was saved.  Rather show some live buffer in
+them and call each function in `window-kept-windows-functions'
+with a list of two arguments: the frame where STATE was put and a
+list of entries for each such window.  Each entry contains four
+elements - the window, its old buffer and the last positions of
+`window-start' and `window-point' for the buffer in that window.
+Always check the window for liveness because another function run
+by this hook may have deleted it."
   (setq window-state-put-stale-windows nil)
+  (setq window-state-put-kept-windows nil)
 
   ;; 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.
@@ -6482,6 +6525,7 @@ window-state-put
 	(error "Window %s too small to accommodate state" window)
       (setq state (cdr state))
       (setq window-state-put-list nil)
+      (setq window-state-put-selected-window nil)
       ;; Work on the windows of a temporary buffer to make sure that
       ;; splitting proceeds regardless of any buffer local values of
       ;; `window-size-fixed'.  Release that buffer after the buffers of
@@ -6490,14 +6534,21 @@ window-state-put
 	(set-window-buffer window (current-buffer))
 	(window--state-put-1 state window nil totals pixelwise)
 	(window--state-put-2 ignore pixelwise))
+      (when (window-live-p window-state-put-selected-window)
+	(select-window window-state-put-selected-window))
       (while window-state-put-stale-windows
 	(let ((window (pop window-state-put-stale-windows)))
-          ;; Avoid that 'window-deletable-p' throws an error if window
+	  ;; Avoid that 'window-deletable-p' throws an error if window
           ;; was already deleted when exiting 'with-temp-buffer' above
           ;; (Bug#54028).
 	  (when (and (window-valid-p window)
                      (eq (window-deletable-p window) t))
 	    (delete-window window))))
+      (when window-kept-windows-functions
+	(run-hook-with-args
+	 'window-kept-windows-functions
+	 frame window-state-put-kept-windows)
+	(setq window-state-put-kept-windows nil))
       (window--check frame))))
 
 (defun window-state-buffers (state)
diff --git a/src/marker.c b/src/marker.c
index 377f6fbe8db..14b9f63f0cd 100644
--- a/src/marker.c
+++ b/src/marker.c
@@ -458,6 +458,18 @@ DEFUN ("marker-position", Fmarker_position, Smarker_position, 1, 1, 0,
   return Qnil;
 }
 
+DEFUN ("marker-last-position", Fmarker_last_position, Smarker_last_position, 1, 1, 0,
+       doc: /* Return last position of MARKER in its buffer.
+This is like `marker-position' with one exception:  If the buffer of
+MARKER is dead, it returns the last position of MARKER in that buffer
+before it was killed.  */)
+  (Lisp_Object marker)
+{
+  CHECK_MARKER (marker);
+
+  return make_fixnum (XMARKER (marker)->charpos);
+}
+
 /* Change M so it points to B at CHARPOS and BYTEPOS.  */
 
 static void
@@ -825,6 +837,7 @@ verify_bytepos (ptrdiff_t charpos)
 syms_of_marker (void)
 {
   defsubr (&Smarker_position);
+  defsubr (&Smarker_last_position);
   defsubr (&Smarker_buffer);
   defsubr (&Sset_marker);
   defsubr (&Scopy_marker);
diff --git a/src/window.c b/src/window.c
index 3a54f7ce7b1..8e002d70db6 100644
--- a/src/window.c
+++ b/src/window.c
@@ -7090,6 +7090,24 @@ DEFUN ("set-window-configuration", Fset_window_configuration,
 the mini-window of the frame doesn't get set to the corresponding element
 of CONFIGURATION.
 
+Normally, this function will try to delete any dead window in
+CONFIGURATION whose buffer has been deleted since CONFIGURATION was
+made.  However, if the abnormal hook `window-kept-windows-functions' is
+non-nil, it will preserve such a window in the restored layout and show
+another buffer in it.
+
+After restoring the frame layout, this function runs the abnormal hook
+`window-kept-windows-functions' with two arguments - the frame whose
+layout it has restored and a list of entries for each window whose
+buffer has been found dead when it tried to restore CONFIGURATION: Each
+entry is a list of four elements <window, buffer, start, point> where
+`window' denotes the window whose buffer was found dead, `buffer'
+denotes the dead buffer, and `start' and `point' denote the last known
+positions of `window-start' and `window-point' of the buffer in that
+window.  Any function run by this hook should check the window for
+liveness because another function run by this hook may have deleted it
+in the meantime."
+
 If CONFIGURATION was made from a frame that is now deleted,
 only frame-independent values can be restored.  In this case,
 the return value is nil.  Otherwise the value is t.  */)
@@ -7100,6 +7118,7 @@ DEFUN ("set-window-configuration", Fset_window_configuration,
   struct Lisp_Vector *saved_windows;
   Lisp_Object new_current_buffer;
   Lisp_Object frame;
+  Lisp_Object kept_windows = Qnil;
   Lisp_Object old_frame = selected_frame;
   struct frame *f;
   ptrdiff_t old_point = -1;
@@ -7340,6 +7359,11 @@ DEFUN ("set-window-configuration", Fset_window_configuration,
 		   BUF_PT (XBUFFER (w->contents)),
 		   BUF_PT_BYTE (XBUFFER (w->contents)));
 	      w->start_at_line_beg = true;
+	      if (!NILP (Vwindow_kept_windows_functions))
+		kept_windows = Fcons (list4 (window, p->buffer,
+					     Fmarker_last_position (p->start),
+					     Fmarker_last_position (p->pointm)),
+				      kept_windows);
 	    }
 	  else if (!NILP (w->start))
 	    /* Leaf window has no live buffer, get one.  */
@@ -7360,6 +7384,11 @@ DEFUN ("set-window-configuration", Fset_window_configuration,
 		dead_windows = Fcons (window, dead_windows);
 	      /* Make sure window is no more dedicated.  */
 	      wset_dedicated (w, Qnil);
+	      if (!NILP (Vwindow_kept_windows_functions))
+		kept_windows = Fcons (list4 (window, p->buffer,
+					     Fmarker_last_position (p->start),
+					     Fmarker_last_position (p->pointm)),
+				      kept_windows);
 	    }
 	}
 
@@ -7411,12 +7440,13 @@ DEFUN ("set-window-configuration", Fset_window_configuration,
       unblock_input ();
 
       /* Scan dead buffer windows.  */
-      for (; CONSP (dead_windows); dead_windows = XCDR (dead_windows))
-	{
-	  window = XCAR (dead_windows);
-	  if (WINDOW_LIVE_P (window) && !EQ (window, FRAME_ROOT_WINDOW (f)))
-	    delete_deletable_window (window);
-	}
+      if (!NILP (Vwindow_kept_windows_functions))
+	for (; CONSP (dead_windows); dead_windows = XCDR (dead_windows))
+	  {
+	    window = XCAR (dead_windows);
+	    if (WINDOW_LIVE_P (window) && !EQ (window, FRAME_ROOT_WINDOW (f)))
+	      delete_deletable_window (window);
+	  }
 
       /* Record the selected window's buffer here.  The window should
 	 already be the selected one from the call above.  */
@@ -7463,6 +7493,11 @@ DEFUN ("set-window-configuration", Fset_window_configuration,
   minibuf_selected_window = data->minibuf_selected_window;
 
   SAFE_FREE ();
+
+  if (!NILP (Vrun_hooks) && !NILP (Vwindow_kept_windows_functions))
+    run_hook_with_args_2 (Qwindow_kept_windows_functions, frame,
+			  kept_windows);
+
   return FRAME_LIVE_P (f) ? Qt : Qnil;
 }
 
@@ -8460,6 +8495,8 @@ syms_of_window (void)
   DEFSYM (Qheader_line_format, "header-line-format");
   DEFSYM (Qtab_line_format, "tab-line-format");
   DEFSYM (Qno_other_window, "no-other-window");
+  DEFSYM (Qwindow_kept_windows_functions,
+	  "window-kept-windows-functions");
 
   DEFVAR_LISP ("temp-buffer-show-function", Vtemp_buffer_show_function,
 	       doc: /* Non-nil means call as function to display a help buffer.
@@ -8617,6 +8654,28 @@ syms_of_window (void)
 call is performed with the frame temporarily selected.  */);
   Vwindow_configuration_change_hook = Qnil;
 
+  DEFVAR_LISP ("window-kept-windows-functions",
+	       Vwindow_kept_windows_functions,
+	       doc: /* Functions run after restoring a window configuration or state.
+These functions are called by `set-window-configuration' and
+`window-state-put'.  When the value of this variable is non-nil, these
+functions restore any window whose buffer has been deleted since the
+corresponding configuration or state was saved.  Rather than deleting
+such a window, `set-window-configuration' and `window-state-put' show
+some live buffer in it.
+
+The value should be a list of functions that take two arguments.  The
+first argument specifies the frame whose configuration has been
+restored.  The second argument, if non-nil, specifies a list of entries
+for each window whose buffer has been found dead at the time
+'set-window-configuration' or `window-state-put' tried to restore it in
+that window.  Each entry is a list of four values - the window whose
+buffer was found dead, the dead buffer, and the positions of start and
+point of the buffer in that window.  Note that the window may be already
+dead since another function on this list may have deleted it in the
+meantime.  */);
+  Vwindow_kept_windows_functions = Qnil;
+
   DEFVAR_LISP ("recenter-redisplay", Vrecenter_redisplay,
 	       doc: /* Non-nil means `recenter' redraws entire frame.
 If this option is non-nil, then the `recenter' command with a nil

^ permalink raw reply related	[flat|nested] 20+ messages in thread

* bug#69093: window-state-put doesn't update current buffer
  2024-02-25  9:17                         ` martin rudalics via Bug reports for GNU Emacs, the Swiss army knife of text editors
@ 2024-02-25 18:19                           ` Juri Linkov
  2024-02-26  8:42                             ` martin rudalics via Bug reports for GNU Emacs, the Swiss army knife of text editors
  0 siblings, 1 reply; 20+ messages in thread
From: Juri Linkov @ 2024-02-25 18:19 UTC (permalink / raw)
  To: martin rudalics; +Cc: 69093

> The attached patch (including the former changes) should fix it.  I left
> the old 'select-window' call in just for the case that its effect is
> used elsewhere in a function called by 'window--state-put-2' later.

Thank you very much.  I confirm that everything works nicely.





^ permalink raw reply	[flat|nested] 20+ messages in thread

* bug#69093: window-state-put doesn't update current buffer
  2024-02-25 18:19                           ` Juri Linkov
@ 2024-02-26  8:42                             ` martin rudalics via Bug reports for GNU Emacs, the Swiss army knife of text editors
  2024-03-04  9:40                               ` martin rudalics via Bug reports for GNU Emacs, the Swiss army knife of text editors
  0 siblings, 1 reply; 20+ messages in thread
From: martin rudalics via Bug reports for GNU Emacs, the Swiss army knife of text editors @ 2024-02-26  8:42 UTC (permalink / raw)
  To: Juri Linkov; +Cc: 69093

 > Thank you very much.  I confirm that everything works nicely.

If no problems come up within a week, I'll try to install it.

Thanks for testing, martin





^ permalink raw reply	[flat|nested] 20+ messages in thread

* bug#69093: window-state-put doesn't update current buffer
  2024-02-26  8:42                             ` martin rudalics via Bug reports for GNU Emacs, the Swiss army knife of text editors
@ 2024-03-04  9:40                               ` martin rudalics via Bug reports for GNU Emacs, the Swiss army knife of text editors
  2024-03-05 17:14                                 ` Juri Linkov
  0 siblings, 1 reply; 20+ messages in thread
From: martin rudalics via Bug reports for GNU Emacs, the Swiss army knife of text editors @ 2024-03-04  9:40 UTC (permalink / raw)
  To: Juri Linkov; +Cc: 69093

All changes should be on master now.

Thanks for the reviews, martin






^ permalink raw reply	[flat|nested] 20+ messages in thread

* bug#69093: window-state-put doesn't update current buffer
  2024-03-04  9:40                               ` martin rudalics via Bug reports for GNU Emacs, the Swiss army knife of text editors
@ 2024-03-05 17:14                                 ` Juri Linkov
  0 siblings, 0 replies; 20+ messages in thread
From: Juri Linkov @ 2024-03-05 17:14 UTC (permalink / raw)
  To: martin rudalics; +Cc: 69093

close 69093 30.0.50
thanks

> All changes should be on master now.

Thanks.  I confirm 'window-state-put' working,
so I was able to add 'tab-bar-tab-post-select-functions'.





^ permalink raw reply	[flat|nested] 20+ messages in thread

end of thread, other threads:[~2024-03-05 17:14 UTC | newest]

Thread overview: 20+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2024-02-13  7:39 bug#69093: window-state-put doesn't update current buffer Juri Linkov
2024-02-14  9:12 ` martin rudalics via Bug reports for GNU Emacs, the Swiss army knife of text editors
2024-02-15  7:29   ` Juri Linkov
2024-02-16  9:39     ` martin rudalics via Bug reports for GNU Emacs, the Swiss army knife of text editors
2024-02-16 11:46       ` Eli Zaretskii
2024-02-18  7:43       ` Juri Linkov
2024-02-19  9:43         ` martin rudalics via Bug reports for GNU Emacs, the Swiss army knife of text editors
2024-02-19 17:36           ` Drew Adams
2024-02-20  7:40           ` Juri Linkov
2024-02-21  9:04             ` martin rudalics via Bug reports for GNU Emacs, the Swiss army knife of text editors
2024-02-21 17:27               ` Juri Linkov
2024-02-22  8:58                 ` martin rudalics via Bug reports for GNU Emacs, the Swiss army knife of text editors
2024-02-22 17:23                   ` Juri Linkov
2024-02-23  8:48                     ` martin rudalics via Bug reports for GNU Emacs, the Swiss army knife of text editors
2024-02-24 17:32                       ` Juri Linkov
2024-02-25  9:17                         ` martin rudalics via Bug reports for GNU Emacs, the Swiss army knife of text editors
2024-02-25 18:19                           ` Juri Linkov
2024-02-26  8:42                             ` martin rudalics via Bug reports for GNU Emacs, the Swiss army knife of text editors
2024-03-04  9:40                               ` martin rudalics via Bug reports for GNU Emacs, the Swiss army knife of text editors
2024-03-05 17:14                                 ` Juri Linkov

Code repositories for project(s) associated with this public inbox

	https://git.savannah.gnu.org/cgit/emacs.git

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).