EXWM/Miniframe: Windows not restored after preview

[Stebalien]

When consult is used in conjunction with selectrum and mini-frame, the window configuration is not restored after previewing then aborting.

For example,

1. Enable mini-frame-mode, selectrum-mode.
2. Run consult-buffer, then preview a different buffer.
3. Hit C-g to quit.

Expectation: I expected the buffer I was editing before running consult-buffer to be selected and focused.

Reality: The last previewed buffer remains selected and focused, nothing is restored.

Wrapping the command in save-window-excursion as in #177 appears to fix the issue, but I'm unsure why this is only an issue with mini-frame-mode and not an issue otherwise.

[Stebalien]

Note: everything is restored if the focused buffer is not changed (e.g., with consult-lines). This is only an issue with commands like consult-ripgrep and consult-buffer.

[minad]

Okay, I think in order to be safe I should use set-buffer in the restore block of consult--preview-position. Originally I used save-excursion or even save-window-excursion as you suggested but stopped using it since I wanted a more fine grained solution.

[Stebalien]

From what I can tell, consult--preview-position isn't used for consult--buffer, consult--buffer is (which doesn't do anything on restore.

I've been able to get this to work with:

diff --git a/consult.el b/consult.el
index 9920f68..9dd1892 100644
--- a/consult.el
+++ b/consult.el
@@ -2674,6 +2674,7 @@ FACE is the face for the candidate."
 
 Depending on the selected item OPEN-BUFFER, OPEN-FILE or OPEN-BOOKMARK will be used to display the item."
   (let* ((curr-buf (current-buffer))
+	 (curr-win (get-buffer-window curr-buf))
          (all-bufs (append (delq curr-buf (buffer-list)) (list curr-buf)))
          (buf-file-hash (let ((ht (make-hash-table :test #'equal :size (length all-bufs))))
                           (dolist (buf all-bufs ht)
@@ -2756,8 +2757,11 @@ Depending on the selected item OPEN-BUFFER, OPEN-FILE or OPEN-BOOKMARK will be u
            :preview
            (lambda (cand restore)
              (cond
-              (restore)
-              ;; In order to avoid slowness and unnecessary complexity, we
+              (restore
+	       (if (not (buffer-live-p curr-buf))
+		   (message "Buffer is dead")
+		 (set-window-buffer curr-win curr-buf)))
+	      ;; In order to avoid slowness and unnecessary complexity, we
               ;; only preview buffers. Loading recent files, bookmarks or
               ;; views can result in expensive operations.
               ((and (or (eq (car cand) #'switch-to-buffer)

However, that's still suboptimal because it only restores the current window. I use EXWM so previewing buffers ends up "stealing" X windows from other windows and they don't get restored after exiting.

What's the motivation for handling this manually? Generally, I'd expect completions to have no side effects other than to return the completion result (and act on it). This just seems like a lot of work to make sure nothing goes wrong when there's a nice big hammer (save-window-excursion) just sitting there.

[minad]

The problem is that save-window-excursion just wraps everything. But not for every consult--read session case I want to do such a full restoration.

However, that's still suboptimal because it only restores the current window. I use EXWM so previewing buffers ends up "stealing" X windows from other windows and they don't get restored after exiting.

Why does this happen? I don't understand what is going on with your setup honestly.

What about consult-grep? This uses consult--preview-position.

[minad]

Why does consult-buffer mess up other windows? Is this expected in your setup? I could easily use save-window-excursion or just use the relevant save/restore code from there, but I don't want to, since consult-buffer should not mess up the window configuration so badly.

Basically the assumption is that when leaving completing-read we are back in the original buffer. This holds in my configuration. If you enable recursive minibuffers and call completing-read, jump to the last buffer and kill it, then you will see the message "Selecting deleted buffer".

(completing-read "Prompt: " '(a b c))

Can you try without miniframe or without exwm to see where the problem lies?

[Stebalien]

Why does this happen? I don't understand what is going on with your setup honestly.

This is just a side effect of EXWM. An X window can only be visible "once", so if a buffer containing an X window is selected in an Emacs window, the buffer containing the X window is removed from all other windows.

Why does consult-buffer mess up other windows?

It doesn't move the windows, it just changes the selected window's current buffer and doesn't restore it.

I'm quite confused here because I can't seem to find the code responsible for restoring the selected window's current buffer (in consult-buffer) but something appears to be doing that in the normal (non mini-frame) case.

[Stebalien]

What about consult-grep? This uses consult--preview-position.

Same issue. Fixed with:

diff --git a/consult.el b/consult.el
index 9920f68..41bc8aa 100644
--- a/consult.el
+++ b/consult.el
@@ -860,6 +860,7 @@ FACE is the cursor face."
   (let ((overlays)
         (invisible)
         (face (or face 'consult-preview-cursor))
+	(saved-win (selected-window))
         (saved-min (point-min-marker))
         (saved-max (point-max-marker))
         (saved-pos (point-marker)))
@@ -871,7 +872,8 @@ FACE is the cursor face."
         (if (not (buffer-live-p (marker-buffer saved-pos)))
             (message "Buffer is dead")
           (narrow-to-region saved-min saved-max)
-          (goto-char saved-pos)))
+          (goto-char saved-pos)
+	  (set-window-buffer saved-win (marker-buffer saved-pos))))
        ;; Jump to position
        (cand
         (consult--jump-1 cand)

[minad]

I'm quite confused here because I can't seem to find the code responsible for restoring the selected window's current buffer (in consult-buffer) but something appears to be doing that in the normal (non mini-frame) case.

If you take a look at the read_minibuf function in the C code - there is some buffer and window configuration saving/restoring going on. I am not sure how mini-frame this worksbut it seems that it breaks the usual behavior of read-from-minibuffer.

[Stebalien]

Can you try without miniframe or without exwm to see where the problem lies?

This is an issue with mini-frame, not EXWM. If I disable mini-frame, run any completion, switch the selected buffer, then exit the completion, the original buffer is restored.

So it would appear that Emacs just does this automatically. And yeah, mini-frame does appear to break this.

I'll try to figure out how mini-frame is breaking this, I'm just not entirely sure if it's fixable...

[Stebalien]

Ah... I found the issue. The solution is save-window-excursion. Just in mini-frame, not here.

[minad]

And yeah, mini-frame does appear to break this. I'll try to figure out how mini-frame is breaking this, I'm just not entirely sure if it's fixable...

Can you please open an issue on the mini-frame tracker and ask there instead. I am not 100% sure if the assumptions I am doing here are correct, as I said - I originally used many more of the big hammers (saving window config and saving buffers) and then I found out that many of these operations were unnecessary. The fewer things I am doing, the easier it is to find problems. Note that Consult also has to interact with Marginalia and Embark which also have to mess with windows in order to execute some code in the correct context. Maybe this explains it a bit better why things are they way they are.

About the EXWM issue, I am unsure if I can do anything about it, since EXWM behaves differently from usual buffers with its no-cloning behavior.

[Stebalien]

You're right, this is something that should be ensured by mini-frame.

The EXWM issue is also fixed by fixing mini-frame to save the window configuration.