Re: Stop frames stealing eachothers' minibuffers!

[Alan Mackenzie <acm@muc.de>]

Hello, Stefan and jakanakaevangeli.

On Thu, Feb 11, 2021 at 09:29:44 -0500, Stefan Monnier wrote:
> > This is more involved.  What do we want to happen when a frame with open
> > minibuffers is deleted?  I would say that these minibuffers should,
> > except in the (eq minibuffer-follows-selected-frame t) case, be aborted,
> > together with any others in other frames whose nesting level makes this
> > necessary.

> I vote for moving those minibuffers elsewhere (anywhere else is fine by
> me, really).  I assume it's no more complicated code-wise, and it should
> suffer less from the risk of losing information.

Just a quick recapitulation of the problem: when
minibuffer-follows-selected-frame is nil, and
enable-recursive-minibuffers t, it is possible to have several open
minibuffers on several frames.  If one of these frames is deleted, its
minibuffers continue to exist, but are wholly inaccessible - the only
thing to do with them is to abort them, e.g. with C-].  This is
suboptimal.

The patch below tries to solve this by, when such a frame gets deleted,
"zipping" its minibuffers into those of another frame.  The idea behind
the patch is to use the mini-window's w->prev_buffers component to hold
the list of its minibuffers.  This mini-window component was unused by
the rest of Emacs, apart from accidentally by
window--before-delete-windows, which I have now amended.

I would be grateful indeed if either of you (or indeed, anybody else),
would apply the patch and try it out, or indeed comment on it.  Thanks!



diff --git a/doc/emacs/mini.texi b/doc/emacs/mini.texi
index f81e64bdf9..7da0a48b7c 100644
--- a/doc/emacs/mini.texi
+++ b/doc/emacs/mini.texi
@@ -82,7 +82,9 @@ Basic Minibuffer
 (@pxref{Recursive Mini,,, elisp}).  This option is mainly to retain
 (approximately) the behavior prior to Emacs 28.1.  Note that the
 effect of the command, when you finally finish using the minibuffer,
-always takes place in the frame where you first opened it.
+always takes place in the frame where you first opened it.  The sole
+exception is that when that frame no longer exists, the action takes
+place in the currently selected frame.
 
 @node Minibuffer File
 @section Minibuffers for File Names
diff --git a/lisp/window.el b/lisp/window.el
index cfd9876ed0..fb2ea4a985 100644
--- a/lisp/window.el
+++ b/lisp/window.el
@@ -4158,7 +4158,7 @@ window--before-delete-windows
 
 This function is called only if `switch-to-buffer-preserve-window-point'
 evaluates non-nil."
-  (dolist (win (window-list))
+  (dolist (win (window-list nil 'no-minibuf))
     (let* ((buf   (window-buffer (or window win)))
            (start (window-start win))
            (pos   (window-point win))
@@ -4416,7 +4416,8 @@ record-window-buffer
        window (assq-delete-all buffer (window-prev-buffers window))))
 
     ;; Don't record insignificant buffers.
-    (unless (eq (aref (buffer-name buffer) 0) ?\s)
+    (when (or (not (eq (aref (buffer-name buffer) 0) ?\s))
+              (string-match "^ \\*Minibuf" (buffer-name buffer)))
       ;; Add an entry for buffer to WINDOW's previous buffers.
       (with-current-buffer buffer
 	(let ((start (window-start window))
diff --git a/src/frame.c b/src/frame.c
index a62347c1fb..b9df5739dd 100644
--- a/src/frame.c
+++ b/src/frame.c
@@ -1487,7 +1487,7 @@ do_switch_frame (Lisp_Object frame, int track, int for_deletion, Lisp_Object nor
 #endif
     internal_last_event_frame = Qnil;
 
-  move_minibuffer_onto_frame ();
+  move_minibuffers_onto_frame (sf, for_deletion);
   return frame;
 }
 
diff --git a/src/lisp.h b/src/lisp.h
index b95f389b89..2f4e6377cb 100644
--- a/src/lisp.h
+++ b/src/lisp.h
@@ -4348,7 +4348,7 @@ extern void clear_regexp_cache (void);
 
 extern Lisp_Object Vminibuffer_list;
 extern Lisp_Object last_minibuf_string;
-extern void move_minibuffer_onto_frame (void);
+extern void move_minibuffers_onto_frame (struct frame *, int);
 extern bool is_minibuffer (EMACS_INT, Lisp_Object);
 extern EMACS_INT this_minibuffer_depth (Lisp_Object);
 extern EMACS_INT minibuf_level;
diff --git a/src/minibuf.c b/src/minibuf.c
index 4b1f4b1ff7..53ed8d5ef8 100644
--- a/src/minibuf.c
+++ b/src/minibuf.c
@@ -67,6 +67,7 @@ static ptrdiff_t minibuf_prompt_width;
 static Lisp_Object nth_minibuffer (EMACS_INT depth);
 static EMACS_INT minibuf_c_loop_level (EMACS_INT depth);
 static void set_minibuffer_mode (Lisp_Object buf, EMACS_INT depth);
+static bool live_minibuffer_p (Lisp_Object);
 
 \f
 /* Return TRUE when a frame switch causes a minibuffer on the old
@@ -78,6 +79,7 @@ minibuf_follows_frame (void)
              Qt);
 }
 
+#if 0
 /* Return TRUE when a minibuffer always remains on the frame where it
    was first invoked. */
 static bool
@@ -85,6 +87,7 @@ minibuf_stays_put (void)
 {
   return NILP (Fdefault_toplevel_value (Qminibuffer_follows_selected_frame));
 }
+#endif
 
 /* Return TRUE when opening a (recursive) minibuffer causes
    minibuffers on other frames to move to the selected frame.  */
@@ -112,83 +115,137 @@ choose_minibuf_frame (void)
 	emacs_abort ();
 
       minibuf_window = sf->minibuffer_window;
-      /* If we've still got another minibuffer open, use its mini-window
-         instead.  */
-      if (minibuf_level > 1 && minibuf_stays_put ())
-        {
-          Lisp_Object buffer = get_minibuffer (minibuf_level);
-          Lisp_Object tail, frame;
-
-          FOR_EACH_FRAME (tail, frame)
-            if (EQ (XWINDOW (XFRAME (frame)->minibuffer_window)->contents,
-                    buffer))
-              {
-                minibuf_window = XFRAME (frame)->minibuffer_window;
-                break;
-              }
-        }
     }
+}
 
-  if (minibuf_moves_frame_when_opened ()
-      && FRAMEP (selected_frame)
-      && FRAME_LIVE_P (XFRAME (selected_frame)))
-    /* Make sure no other frame has a minibuffer as its selected window,
-       because the text would not be displayed in it, and that would be
-       confusing.  Only allow the selected frame to do this,
-       and that only if the minibuffer is active.  */
-  {
-    Lisp_Object tail, frame;
-    struct frame *of;
-
-    FOR_EACH_FRAME (tail, frame)
-      if (!EQ (frame, selected_frame)
-          && minibuf_level > 1
-	  /* The frame's minibuffer can be on a different frame.  */
-	  && ! EQ (XWINDOW ((of = XFRAME (frame))->minibuffer_window)->frame,
-		   selected_frame))
-        {
-          if (MINI_WINDOW_P (XWINDOW (FRAME_SELECTED_WINDOW (of))))
-            Fset_frame_selected_window (frame, Fframe_first_window (frame),
-                                        Qnil);
-
-          if (!EQ (XWINDOW (of->minibuffer_window)->contents,
-                   nth_minibuffer (0)))
-            set_window_buffer (of->minibuffer_window,
-                               nth_minibuffer (0), 0, 0);
-        }
-  }
+/* Get the next live buffer entry from a w->prev_buffer, setting the pertinent
+  variables of `zip_minibuffer_stacks'.  The parameter P is either "d" for
+  the destination structures, or "s" for the source structures.  When the
+  ->prev_buffers list is exhausted, set di/si to -1.  */
+#define NEXT_BUFFER_ENTRY(p)						\
+  do									\
+    {									\
+      if (NILP (p##_bufs))						\
+        {								\
+	  p##b = Qnil;							\
+	  p##_ent = Qnil;						\
+	}								\
+      else								\
+	{								\
+	  p##_ent = Fcar (p##_bufs);					\
+	  p##b = Fcar (p##_ent);					\
+	  p##_bufs = Fcdr (p##_bufs);					\
+	}								\
+      if (!NILP (p##b))							\
+	p##i = this_minibuffer_depth (p##b);				\
+      else								\
+	p##i = -1;							\
+    } while (p##i == 0)
+
+/* Move the ordered "stack" of minibuffers from SOURCE_WINDOW to
+   DEST_WINDOW, interleaving those minibuffers with any in DEST_WINDOW
+   to produce an ordered combination.  The ordering is by minibuffer
+   depth.  A stack of minibuffers consists of the minibuffer currently
+   in DEST/SOURCE_WINDOW together with any recorded in the
+   ->prev_buffers field of the struct window.  */
+static void
+zip_minibuffer_stacks (Lisp_Object dest_window, Lisp_Object source_window)
+{
+  struct window *dw = XWINDOW (dest_window);
+  struct window *sw = XWINDOW (source_window);
+  Lisp_Object d_bufs, s_bufs;	/* Lists of minibuffer entries */
+  Lisp_Object acc = Qnil;
+  Lisp_Object d_ent, s_ent;	/* Entries from dw/sw->prev_buffers */
+  Lisp_Object db, sb;		/* (Mini)buffers from the above */
+  EMACS_INT di, si;  /* Indices in the minibuffer list of db and sb */
+
+  if (!live_minibuffer_p (dw->contents)
+      && NILP (dw->prev_buffers))
+    {
+      set_window_buffer (dest_window, sw->contents, 0, 0);
+      Fset_window_start (dest_window, Fwindow_start (source_window), Qnil);
+      Fset_window_point (dest_window, Fwindow_point (source_window));
+      dw->prev_buffers = sw->prev_buffers;
+      set_window_buffer (source_window, get_minibuffer (0), 0, 0);
+      sw->prev_buffers = Qnil;
+      return;
+    }
+
+  if (live_minibuffer_p (dw->contents))
+    call1 (Qrecord_window_buffer, dest_window);
+  if (live_minibuffer_p (sw->contents))
+    call1 (Qrecord_window_buffer, source_window);
+
+  d_bufs = Fnreverse (dw->prev_buffers);
+  s_bufs = Fnreverse (sw->prev_buffers);
+
+  NEXT_BUFFER_ENTRY (d);
+  NEXT_BUFFER_ENTRY (s);
+
+  while (di != -1 && si != -1)
+    if (di < si)
+      {
+	acc = Fcons (d_ent, acc);
+	NEXT_BUFFER_ENTRY (d);
+      }
+    else
+      {
+	acc = Fcons (s_ent, acc);
+	NEXT_BUFFER_ENTRY (s);
+      }
+  while (di != -1)
+    {
+      acc = Fcons (d_ent, acc);
+      NEXT_BUFFER_ENTRY (d);
+    }
+  while (si != -1)
+    {
+      acc = Fcons (s_ent, acc);
+      NEXT_BUFFER_ENTRY (s);
+    }
+  if (!NILP (acc))
+    {
+      d_ent = Fcar (acc);
+      acc = Fcdr (acc);
+      set_window_buffer (dest_window, Fcar (d_ent), 0, 0);
+      Fset_window_start (dest_window, Fcar (Fcdr (d_ent)), Qnil);
+      Fset_window_point (dest_window, Fcar (Fcdr (Fcdr (d_ent))));
+    }
+  dw->prev_buffers = acc;
+  sw->prev_buffers = Qnil;
+  set_window_buffer (source_window, get_minibuffer (0), 0, 0);
 }
+#undef NEXT_BUFFER_ENTRY
 
-/* If `minibuffer_follows_selected_frame' is t and we have a
-   minibuffer, move it from its current frame to the selected frame.
-   This function is intended to be called from `do_switch_frame' in
-   frame.c.  */
-void move_minibuffer_onto_frame (void)
+/* If `minibuffer_follows_selected_frame' is t, or we're about to
+   delete a frame which potentially "contains" minibuffers, move them
+   from the old frame to the selected frame.  This function is
+   intended to be called from `do_switch_frame' in frame.c.  OF is the
+   old frame, FOR_DELETION is self explanatory.  */
+void
+move_minibuffers_onto_frame (struct frame *of, int for_deletion)
 {
-  if (!minibuf_level)
-    return;
-  if (!minibuf_follows_frame ())
+  if (!for_deletion && (!minibuf_level || !minibuf_follows_frame ()))
     return;
   if (FRAMEP (selected_frame)
       && FRAME_LIVE_P (XFRAME (selected_frame))
-      && !EQ (minibuf_window, XFRAME (selected_frame)->minibuffer_window))
+      && (for_deletion
+	  || !EQ (minibuf_window,
+		  XFRAME (selected_frame)->minibuffer_window)))
     {
-      EMACS_INT i;
       struct frame *sf = XFRAME (selected_frame);
-      Lisp_Object old_frame = XWINDOW (minibuf_window)->frame;
-      struct frame *of = XFRAME (old_frame);
-
-      /* Stack up all the (recursively) open minibuffers on the selected
-         mini_window.  */
-      for (i = 1; i <= minibuf_level; i++)
-	set_window_buffer (sf->minibuffer_window, nth_minibuffer (i), 0, 0);
-      minibuf_window = sf->minibuffer_window;
-      if (of != sf)
+      Lisp_Object mini_frame = XWINDOW (minibuf_window)->frame;
+      struct frame *mf = XFRAME (mini_frame);
+      if (minibuf_follows_frame () || for_deletion)
+	zip_minibuffer_stacks (sf->minibuffer_window,
+			       of->minibuffer_window);
+      if (minibuf_window != sf->minibuffer_window)
 	{
 	  Lisp_Object temp = get_minibuffer (0);
 
-	  set_window_buffer (of->minibuffer_window, temp, 0, 0);
+	  set_window_buffer (mf->minibuffer_window, temp, 0, 0);
 	  set_minibuffer_mode (temp, 0);
+	  minibuf_window = sf->minibuffer_window;
 	}
     }
 }
@@ -221,6 +278,7 @@ without invoking the usual minibuffer commands.  */)
 /* Actual minibuffer invocation.  */
 
 static void read_minibuf_unwind (void);
+static void minibuffer_unwind (void);
 static void run_exit_minibuf_hook (void);
 
 
@@ -544,7 +602,7 @@ read_minibuf (Lisp_Object map, Lisp_Object initial, Lisp_Object prompt,
   Lisp_Object histval;
 
   Lisp_Object empty_minibuf;
-  Lisp_Object dummy, frame;
+  Lisp_Object old_minibuf_window = minibuf_window;
 
   specbind (Qminibuffer_default, defalt);
   specbind (Qinhibit_read_only, Qnil);
@@ -626,17 +684,23 @@ read_minibuf (Lisp_Object map, Lisp_Object initial, Lisp_Object prompt,
   mini_frame = WINDOW_FRAME (XWINDOW (minibuf_window));
 
   if (minibuf_level > 1
+      && (!EQ (minibuf_window, old_minibuf_window))
       && minibuf_moves_frame_when_opened ()
-      && (!minibuf_follows_frame ()
-	  || (!EQ (mini_frame, selected_frame))))
+      && (!minibuf_follows_frame ()))
     {
-      EMACS_INT i;
+      Lisp_Object old_frame = XWINDOW (old_minibuf_window)->frame;
+      struct frame *of = XFRAME (old_frame);
 
-      /* Stack up the existing minibuffers on the current mini-window */
-      for (i = 1; i < minibuf_level; i++)
-	set_window_buffer (minibuf_window, nth_minibuffer (i), 0, 0);
+      zip_minibuffer_stacks (minibuf_window, old_minibuf_window);
+      /* The 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);
     }
+  if (live_minibuffer_p (XWINDOW (minibuf_window)->contents))
+    call1 (Qrecord_window_buffer, minibuf_window);
 
+  record_unwind_protect_void (minibuffer_unwind);
   record_unwind_protect (restore_window_configuration,
                          Fcons (Qt, Fcurrent_window_configuration (Qnil)));
 
@@ -771,23 +835,6 @@ read_minibuf (Lisp_Object map, Lisp_Object initial, Lisp_Object prompt,
   empty_minibuf = get_minibuffer (0);
   set_minibuffer_mode (empty_minibuf, 0);
 
-  FOR_EACH_FRAME (dummy, frame)
-    {
-      Lisp_Object root_window = Fframe_root_window (frame);
-      Lisp_Object mini_window = XWINDOW (root_window)->next;
-      Lisp_Object buffer;
-
-      if (!NILP (mini_window) && !EQ (mini_window, minibuf_window)
-          && !NILP (Fwindow_minibuffer_p (mini_window)))
-        {
-          buffer = XWINDOW (mini_window)->contents;
-          if (!live_minibuffer_p (buffer))
-            /* Use set_window_buffer instead of Fset_window_buffer (see
-               discussion of bug#11984, bug#12025, bug#12026).  */
-            set_window_buffer (mini_window, empty_minibuf, 0, 0);
-        }
-    }
-
   /* Display this minibuffer in the proper window.  */
   /* Use set_window_buffer instead of Fset_window_buffer (see
      discussion of bug#11984, bug#12025, bug#12026).  */
@@ -908,7 +955,9 @@ read_minibuf (Lisp_Object map, Lisp_Object initial, Lisp_Object prompt,
   unbind_to (count, Qnil);
 
   /* Switch the frame back to the calling frame.  */
-  if (!EQ (selected_frame, calling_frame)
+  if ((!EQ (selected_frame, calling_frame)
+       || !EQ (XWINDOW (XFRAME (calling_frame)->minibuffer_window)->frame,
+	       calling_frame))
       && FRAMEP (calling_frame)
       && FRAME_LIVE_P (XFRAME (calling_frame)))
     call2 (intern ("select-frame-set-input-focus"), calling_frame, Qnil);
@@ -1127,20 +1176,57 @@ read_minibuf_unwind (void)
      away from the expired minibuffer window, both in the current
      minibuffer's frame and the original calling frame.  */
   choose_minibuf_frame ();
-  if (!EQ (WINDOW_FRAME (XWINDOW (minibuf_window)), calling_frame))
-  {
-    Lisp_Object prev = Fprevious_window (minibuf_window, Qnil, Qnil);
-    /* PREV can be on a different frame when we have a minibuffer only
-       frame, the other frame's minibuffer window is MINIBUF_WINDOW,
-       and its "focus window" is also MINIBUF_WINDOW.  */
-    if (!EQ (prev, minibuf_window)
-	&& EQ (WINDOW_FRAME (XWINDOW (prev)),
-	       WINDOW_FRAME (XWINDOW (minibuf_window))))
-      Fset_frame_selected_window (selected_frame, prev, Qnil);
-  }
-  else
-    Fset_frame_selected_window (calling_frame, calling_window, Qnil);
+  if (NILP (XWINDOW (minibuf_window)->prev_buffers))
+    {
+      if (!EQ (WINDOW_FRAME (XWINDOW (minibuf_window)), calling_frame))
+	{
+	  Lisp_Object prev = Fprevious_window (minibuf_window, Qnil, Qnil);
+	  /* PREV can be on a different frame when we have a minibuffer only
+	     frame, the other frame's minibuffer window is MINIBUF_WINDOW,
+	     and its "focus window" is also MINIBUF_WINDOW.  */
+	  if (!EQ (prev, minibuf_window)
+	      && EQ (WINDOW_FRAME (XWINDOW (prev)),
+		     WINDOW_FRAME (XWINDOW (minibuf_window))))
+	    Fset_frame_selected_window (selected_frame, prev, Qnil);
+	}
+      else
+	Fset_frame_selected_window (calling_frame, calling_window, Qnil);
+    }
 }
+
+/* Replace the expired minibuffer in the selected frame with the next
+   less nested minibuffer, if any, stored in the minibuffer window.
+   Otherwise, replace it with the null minibuffer.  MINIBUF_WINDOW
+   gets set to the selected frame's minibuffer.  */
+static void
+minibuffer_unwind (void)
+{
+  struct frame *sf = XFRAME (selected_frame);
+  struct window *w;
+  Lisp_Object entry;
+
+  if (FRAMEP (selected_frame)
+      && FRAME_LIVE_P (sf))
+    {
+      minibuf_window = sf->minibuffer_window;
+      w = XWINDOW (minibuf_window);
+      if (!NILP (w->prev_buffers))
+	{
+	  entry = Fcar (w->prev_buffers);
+	  w->prev_buffers = Fcdr (w->prev_buffers);
+	  set_window_buffer (minibuf_window, Fcar (entry), 0, 0);
+	  Fset_window_start (minibuf_window, Fcar (Fcdr (entry)), Qnil);
+	  Fset_window_point (minibuf_window, Fcar (Fcdr (Fcdr (entry))));
+	  /* set-window-configuration may/will have unselected the
+	     mini-window as the selected window.  Restore it. */
+	  if (EQ (w->frame, selected_frame))
+	    Fset_frame_selected_window (selected_frame, minibuf_window, Qnil);
+	}
+      else
+	set_window_buffer (minibuf_window, nth_minibuffer (0), 0, 0);
+    }
+}
+
 \f
 
 void
diff --git a/src/window.c b/src/window.c
index eb16e2a433..cde53e8059 100644
--- a/src/window.c
+++ b/src/window.c
@@ -6958,7 +6960,8 @@ the return value is nil.  Otherwise the value is t.  */)
 
 	  if (BUFFERP (w->contents)
 	      && !EQ (w->contents, p->buffer)
-	      && BUFFER_LIVE_P (XBUFFER (p->buffer)))
+	      && BUFFER_LIVE_P (XBUFFER (p->buffer))
+	      && (NILP (Fminibufferp (p->buffer, Qnil))))
 	    /* If a window we restore gets another buffer, record the
 	       window's old buffer.  */
 	    call1 (Qrecord_window_buffer, window);
diff --git a/src/xdisp.c b/src/xdisp.c
index cc0a689ba3..a405d51f80 100644
--- a/src/xdisp.c
+++ b/src/xdisp.c
@@ -12650,9 +12650,8 @@ gui_consider_frame_title (Lisp_Object frame)
 	 mode_line_noprop_buf; then display the title.  */
       record_unwind_protect (unwind_format_mode_line,
 			     format_mode_line_unwind_data
-			       (f, current_buffer, selected_window, false));
+			     (NULL, current_buffer, Qnil, false));
 
-      Fselect_window (f->selected_window, Qt);
       set_buffer_internal_1
 	(XBUFFER (XWINDOW (f->selected_window)->contents));
       fmt = FRAME_ICONIFIED_P (f) ? Vicon_title_format : Vframe_title_format;


>         Stefan


-- 
Alan Mackenzie (Nuremberg, Germany).