Re: Stop frames stealing eachothers' minibuffers!

[Alan Mackenzie <acm@muc.de>]

Hello, Miha.

On Sun, Mar 14, 2021 at 22:23:02 +0100, Miha Rihtaršič wrote:
> Alan Mackenzie <acm@muc.de> writes:

> > Hello, Jakanakaevangeli,

> > On Sat, Mar 13, 2021 at 21:53:05 +0100, jakanakaevangeli wrote:

> > [ .... ]

> >> 3)
> >> In emacs daemon, evaluate
> >> (run-at-time 5 nil #'make-frame '((window-system . x)))
> >> and then open a minibuffer and close the last frame. When a new frame
> >> appears, the same problem as in 1) occurs.

> > I think one of the two previous changes fixed this.

> Sorry, this still doesn't seem to be fixed. Just in case I wasn't clear
> enough, I'll try to reiterate.

> When a frame is closed, it's active minibuffer should now be moved to
> another frame. What I wanted to test then was, what happens if we have
> only a single frame with a minibuffer and we decide to close it, which
> is possible with emacs daemon. Immediately, there will be no frames left
> for the minibuffer to take refuge in and this seems to cause some
> problems.

My apologies.  I hadn't understood what the bug is, and can confirm it's
still there.  My patch below doesn't address it.

This could be difficult to fix.  I don't think that clicking on the last
frame's close button goes through `delete-frame' - it just closes the
program, whether that's emacs or emacsclient.  Maybe there's some "close
program" hook that could be used to save any stack of open minibuffers.
I just don't know the code in this area.  At least, not yet.  ;-)

> And two new regressions. These require minibuffer-follows-selected-frame
> to be set to t.

Yes.  Sorry about these.  This time around, I've tried to be more
careful and done more testing myself - hence me taking a few days longer
than I have up till now.  I've found and corrected another ~two bugs
myself.

> 1)
> C-x C-f on frame A
> select frame B
> select frame A
> Minibuffer is moved to B, but not back to A.

> 2)
> Have two frames open
> open a minibuffer on a frame
> close this frame
> The other frame does have the miniwindow selected, but the
> minibuffer isn't shown in it.

I think I've corrected these two, now.

Here's another patch, this time to be applied on top of the last patch.



--- minibuf.20210315.see	2021-03-16 10:36:40.058109894 +0000
+++ minibuf.c	2021-03-17 19:15:33.586176135 +0000
@@ -59,6 +59,12 @@
 
 static Lisp_Object minibuf_prompt;
 
+/* The frame containinug the most recently opened Minibuffer.  This is
+   used only when `minibuffer-follows-selected-frame' is neither nil
+   nor t.  */
+
+static Lisp_Object MB_frame;
+
 /* Width of current mini-buffer prompt.  Only set after display_line
    of the line that contains the prompt.  */
 
@@ -182,19 +188,17 @@
 void
 move_minibuffers_onto_frame (struct frame *of, bool for_deletion)
 {
+  struct frame *f = XFRAME (selected_frame);
+
+  minibuf_window = f->minibuffer_window;
   if (!for_deletion && (!minibuf_level || !minibuf_follows_frame ()))
     return;
-  if (FRAMEP (selected_frame)
-      && FRAME_LIVE_P (XFRAME (selected_frame))
-      && (for_deletion
-	  || !EQ (minibuf_window,
-		  XFRAME (selected_frame)->minibuffer_window)))
+  if (FRAME_LIVE_P (f)
+      && !EQ (f->minibuffer_window, of->minibuffer_window))
     {
-      struct frame *sf = XFRAME (selected_frame);
-
-      if (minibuf_follows_frame () || for_deletion)
-	zip_minibuffer_stacks (sf->minibuffer_window,
-			       of->minibuffer_window);
+      zip_minibuffer_stacks (f->minibuffer_window, of->minibuffer_window);
+      if (for_deletion && EQ (XFRAME (MB_frame), of))
+	MB_frame = selected_frame;
     }
 }
 
@@ -550,7 +554,6 @@
   Lisp_Object histval;
 
   Lisp_Object empty_minibuf;
-  Lisp_Object old_minibuf_window = minibuf_window;
 
   specbind (Qminibuffer_default, defalt);
   specbind (Qinhibit_read_only, Qnil);
@@ -632,19 +635,20 @@
   mini_frame = WINDOW_FRAME (XWINDOW (minibuf_window));
 
   if (minibuf_level > 1
-      && (!EQ (minibuf_window, old_minibuf_window))
+      && !EQ (XWINDOW (XFRAME (selected_frame)->minibuffer_window)->frame,
+	      MB_frame)
       && minibuf_moves_frame_when_opened ()
       && (!minibuf_follows_frame ()))
     {
-      Lisp_Object old_frame = XWINDOW (old_minibuf_window)->frame;
-      struct frame *of = XFRAME (old_frame);
+      struct frame *of = XFRAME (MB_frame);
 
-      zip_minibuffer_stacks (minibuf_window, old_minibuf_window);
-      /* The frame's minibuffer can be on a different frame.  */
+      zip_minibuffer_stacks (minibuf_window, of->minibuffer_window);
+      /* MB_frame's minibuffer can be on a different frame.  */
       if (MINI_WINDOW_P (XWINDOW (FRAME_SELECTED_WINDOW (of))))
-	Fset_frame_selected_window (old_frame,
-				    Fframe_first_window (old_frame), Qnil);
+	Fset_frame_selected_window (MB_frame,
+				    Fframe_first_window (MB_frame), Qnil);
     }
+  MB_frame = XWINDOW (XFRAME (selected_frame)->minibuffer_window)->frame;
   if (live_minibuffer_p (XWINDOW (minibuf_window)->contents))
     call1 (Qrecord_window_buffer, minibuf_window);
 
@@ -1023,10 +1027,13 @@
   safe_run_hooks (Qminibuffer_exit_hook);
 }
 
-/* This variable preserves the minibuffer in the selected frame across
-   the call of restore_window_configuration.  It should be used only
-   by `read_minibuf_unwind' and `minibuffer_unwind'.  */
-static Lisp_Object selected_frame_MB;
+/* This variable records the expired minibuffer's frame between the
+   calls of `read_minibuf_unwind' and `minibuffer_unwind'.  It should
+   be used only by these two functions.  Note that the same search
+   method for the MB's frame won't always work in `minibuffer_unwind'
+   because the intervening `restore-window-configuration' will have
+   changed the buffer in the mini-window.  */
+static Lisp_Object exp_MB_frame;
 
 /* This function is called on exiting minibuffer, whether normally or
    not, and it restores the current window, buffer, etc.  */
@@ -1038,6 +1045,28 @@
   Lisp_Object calling_frame;
   Lisp_Object calling_window;
   Lisp_Object future_mini_window;
+  Lisp_Object saved_selected_frame = selected_frame;
+  Lisp_Object window, frames;
+  struct window *w;
+  struct frame *f;
+
+  /* Locate the expired minibuffer.  */
+  FOR_EACH_FRAME (frames, exp_MB_frame)
+    {
+      f = XFRAME (exp_MB_frame);
+      window = f->minibuffer_window;
+      w = XWINDOW (window);
+      if (EQ (w->frame, exp_MB_frame)
+	  && EQ (w->contents, nth_minibuffer (minibuf_level)))
+	goto found;
+    }
+  return; /* expired minibuffer not found.  Maybe we should output an
+	     error, here. */
+
+ found:
+  if (!EQ (exp_MB_frame, saved_selected_frame))
+    do_switch_frame (exp_MB_frame, 0, 0, Qt); /* This also sets
+					     minibuff_window */
 
   /* To keep things predictable, in case it matters, let's be in the
      minibuffer when we reset the relevant variables.  Don't depend on
@@ -1129,7 +1158,6 @@
      away from the expired minibuffer window, both in the current
      minibuffer's frame and the original calling frame.  */
   choose_minibuf_frame ();
-  selected_frame_MB = XWINDOW (minibuf_window)->contents;
   if (NILP (XWINDOW (minibuf_window)->prev_buffers))
     {
       if (!EQ (WINDOW_FRAME (XWINDOW (minibuf_window)), calling_frame))
@@ -1146,36 +1174,26 @@
       else
 	Fset_frame_selected_window (calling_frame, calling_window, Qnil);
     }
+
+  /* Restore the selected frame. */
+  if (!EQ (exp_MB_frame, saved_selected_frame))
+    do_switch_frame (saved_selected_frame, 0, 0, Qt);
 }
 
-/* Replace the expired minibuffer in whatever frame it is now in with
-   the next less nested minibuffer in that frame, if any.  Otherwise,
-   replace it with the null minibuffer.  MINIBUF_WINDOW is not
-   changed.  */
+/* Replace the expired minibuffer in frame exp_MB_frame with the next less
+   nested minibuffer in that frame, if any.  Otherwise, replace it
+   with the null minibuffer.  MINIBUF_WINDOW is not changed.  */
 static void
 minibuffer_unwind (void)
 {
   struct frame *f;
   struct window *w;
-  Lisp_Object window, frame, frames;
+  Lisp_Object window;
   Lisp_Object entry;
 
-  /* Locate the expired minibuffer.  */
-  FOR_EACH_FRAME (frames, frame)
-    {
-      f = XFRAME (frame);
-      window = f->minibuffer_window;
-      w = XWINDOW (window);
-      if (EQ (w->frame, frame)
-	  && EQ (EQ (frame, selected_frame)
-		 ? selected_frame_MB
-		 : w->contents,
-		 nth_minibuffer (minibuf_level + 1)))
-	goto found;
-    }
-  return; 			/* expired minibuffer not found */
-
- found:
+  f = XFRAME (exp_MB_frame);
+  window = f->minibuffer_window;
+  w = XWINDOW (window);
   if (FRAME_LIVE_P (f))
     {
       /* minibuf_window = sf->minibuffer_window; */
@@ -1188,7 +1206,7 @@
 	  Fset_window_point (window, Fcar (Fcdr (Fcdr (entry))));
 	  /* set-window-configuration may/will have unselected the
 	     mini-window as the selected window.  Restore it. */
-	  Fset_frame_selected_window (frame, window, Qnil);
+	  Fset_frame_selected_window (exp_MB_frame, window, Qnil);
 	}
       else
 	set_window_buffer (window, nth_minibuffer (0), 0, 0);
@@ -2267,7 +2285,9 @@
 {
   staticpro (&minibuf_prompt);
   staticpro (&minibuf_save_list);
-  staticpro (&selected_frame_MB);
+  staticpro (&MB_frame);
+  MB_frame = Qnil;
+  staticpro (&exp_MB_frame);
 
   DEFSYM (Qminibuffer_follows_selected_frame,
           "minibuffer-follows-selected-frame");


> Best.

-- 
Alan Mackenzie (Nuremberg, Germany).